pybind11 icon indicating copy to clipboard operation
pybind11 copied to clipboard

Enforce find_package(Python)

Open rhaschke opened this issue 4 months ago • 4 comments

Fixes #5813

rhaschke avatar Aug 24 '25 11:08 rhaschke

We don't want to call FindPython if it's already been called with the right components present. Or if the user is using the old PythonInterp/Libs support. So I think the check can't be removed, but should instead check for everything that is needed, and trigger the find if everything isn't there.

henryiii avatar Sep 08 '25 14:09 henryiii

We don't want to call FindPython if it's already been called with the right components present.

What's the problem in doing so? Do you fear performance issues? As far a I know, cmake's find functions cache previous results. So that shouldn't be a big deal. On the other hand, find_package might have been called with different search-path arguments before, yielding a Python version < 3.8, which would not be compatible to pybind11. I think, pybind11 should ensure its required version and components itself.

Or if the user is using the old PythonInterp/Libs support.

I think this is not relevant here, as the old mode is handled in pybind11Tools.cmake, not pybind11NewTools.cmake.

So I think the check can't be removed, but should instead check for everything that is needed, and trigger the find if everything isn't there.

This would require moving the logic to figure out the required components out of the if.

rhaschke avatar Sep 08 '25 15:09 rhaschke

cmake's find functions cache previous results

FindPython is the exception (see Python_ARTIFACTS_INTERACTIVE, 3.18+). It was trying to do some weird things, like finding multiple different versions of Python in one build, so it's a really weird find module.

henryiii avatar Dec 03 '25 06:12 henryiii

I'm not sure about the consequences of your statement, @henryiii. Do you still have objections to call find_package(Python) again? If so, how do you suggest to fix issue #5813?

rhaschke avatar Dec 03 '25 09:12 rhaschke