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

fix(cmake): allow forcing old FindPython #5042

Merged
merged 3 commits into from
Mar 21, 2024

Conversation

henryiii
Copy link
Collaborator

@henryiii henryiii commented Feb 27, 2024

Description

Currently, if for some reason the new FindPython triggers, like if Python has been found the new way, you can't get pybind11 to use the old mechanism. If PYBIND11_FINDPYTHON is both defined and falsy, let's use that to force it off.

Suggested changelog entry:

* Setting ``PYBIND11_FINDPYTHON`` to OFF will force the old FindPythonLibs mechanism to be used.

OR Python_FOUND
OR Python2_FOUND
OR Python3_FOUND)
NOT (DEFINED PYBIND11_FINDPYTHON AND NOT PYBIND11_FINDPYTHON)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the long combination of conditions difficult to parse/understand.

What do you think about:

  1. Tiny rewrite of the new additions:
(NOT DEFINED PYBIND11_FINDPYTHON OR PYBIND11_FINDPYTHON)
  1. Adding a comment, similar to what you wrote in the PR description?

henryiii and others added 3 commits March 21, 2024 02:28
@henryiii henryiii force-pushed the henryiii/fix/oldpython branch from b1f9ae2 to 864aadb Compare March 21, 2024 06:28
@henryiii henryiii merged commit ddb8b67 into pybind:master Mar 21, 2024
84 checks passed
@henryiii henryiii deleted the henryiii/fix/oldpython branch March 21, 2024 07:23
@henryiii henryiii added needs changelog Possibly needs a changelog entry and removed needs changelog Possibly needs a changelog entry labels Mar 27, 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.

3 participants