Skip to content

Conversation

@zachcran
Copy link
Contributor

@zachcran zachcran commented Jun 9, 2025

Is this pull request associated with an issue(s)?
No.

Description
The current name used to reference the PyBind11 headers target (pybind11_headers) only exists if PyBind11 is built with your project. If it is already installed, the exported target is instead pybind11::headers. During the build, pybind11_headers is aliased to pybind11::headers (here), so using pybind11::headers exclusively should work in both scenarios.

TODOs

  • ParallelZone may need to be updated first for this to pass.

@zachcran zachcran requested a review from ryanmrichard June 9, 2025 15:43
@zachcran zachcran self-assigned this Jun 9, 2025
@zachcran zachcran added the bug-fix Fixes a bug with a PR label Jun 9, 2025
@zachcran zachcran requested a review from jwaldrop107 June 9, 2025 15:43
@zachcran
Copy link
Contributor Author

zachcran commented Jun 9, 2025

ParallelZone uses nwx_add_pybind11_module() from the NWXCMake repository, while this repo does not. nwx_add_pybind11_module() is still finding the incorrect pybind11_headers, so I think these CI passing tests are potentially a false positive.

I would advise against merging this until:

ryanmrichard
ryanmrichard previously approved these changes Jun 9, 2025
@ryanmrichard
Copy link
Member

I'll probably forget about this PR while you work on the other issues. Please ping us again when this is r2g.

jwaldrop107
jwaldrop107 previously approved these changes Jun 9, 2025
@zachcran zachcran dismissed stale reviews from jwaldrop107 and ryanmrichard via 2888629 June 9, 2025 20:01
@zachcran
Copy link
Contributor Author

zachcran commented Jun 9, 2025

@ryanmrichard @jwaldrop107 This should be r2g now. I made another commit updating a comment to make the tests rerun.

@ryanmrichard ryanmrichard merged commit fbbb59e into master Jun 9, 2025
5 checks passed
@ryanmrichard ryanmrichard deleted the fix_pybind11_tgt_name branch June 9, 2025 20:14
@jwaldrop107
Copy link
Member

🚀 [bumpr] Bumped!
New version:v1.0.44
Changes:v1.0.43...v1.0.44

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix Fixes a bug with a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants