cmake: Use any available Python_add_library command

[dannf]

Description

python_add_library and python3_add_library are mis-cased. These commands are capitalized.

Even when corrected, this code will fail if Python_FOUND is set, but Python_add_library() is not available, which appears to be the issue described here:

#3996

This seems to be due to a mix of FindPython and FindPython3 usage. Both Python_add_library() and Python3_add_library() support Python 3.x, so use either one.

Suggested changelog entry:

pybind11NewTools.cmake: Use available Python(3)_add_library command

[dannf]

Note: I'm not a cmake expert, this just seems to WFM.

[henryiii]

Commands in CMake are case insensitive, only variables are case sensitive. If Python_FOUND is set, Python_add_library should always be defined. Something is wrong if Python was found but only Python3_add_library is present. And Python3_add_library will expect the Python3_* variables to be set, so I think this would just be moving the error.

[henryiii]

Do you have an example of code that this fixes? I'd be curious to see it.

[dannf]

Commands in CMake are case insensitive, only variables are case sensitive.

TIL :)

If Python_FOUND is set, Python_add_library should always be defined. Something is wrong if Python was found but only Python3_add_library is present. And Python3_add_library will expect the Python3_* variables to be set, so I think this would just be moving the error.

I see - thanks!

[henryiii]

Command names are case-insensitive.

https://cmake.org/cmake/help/latest/manual/cmake-language.7.html#command-invocations

(Most commands are actually defined in upper case, actually, including this one)

[henryiii]

If you have an example of a broken CMakeLists.txt, I can take a look.

[dannf]

Do you have an example of code that this fixes? I'd be curious to see it.

Sure. Here's the entry point:
https://github.com/NVIDIA/TensorRT-LLM/blob/28fb9aacaa5a05494635194a9cbb264da9a744bd/cpp/CMakeLists.txt

I generated a cmake trace to try and understand what is going on:
trace.txt

What I noticed there is a windy passage involving mixed FindPython3 and FindPython.

[henryiii]

I think this line: https://github.com/NVIDIA/TensorRT-LLM/blob/28fb9aacaa5a05494635194a9cbb264da9a744bd/cpp/CMakeLists.txt#L473 or https://github.com/NVIDIA/TensorRT-LLM/blob/28fb9aacaa5a05494635194a9cbb264da9a744bd/cpp/CMakeLists.txt#L431 is finding Python3, but pybind11 probably was added before this at https://github.com/NVIDIA/TensorRT-LLM/blob/5bdf997963365ff6ff167e21ab1f53631cf274eb/cpp/CMakeLists.txt#L391, so it found Python. Moving the pybind11 inclusion to after this line would fix it, or pybind11's discovery could be used without re-finding it. As it is, the two discoveries could find different Pythons.

Also it should not be calling python3 before searching for it at https://github.com/NVIDIA/TensorRT-LLM/blob/5bdf997963365ff6ff167e21ab1f53631cf274eb/cpp/CMakeLists.txt#L156.

[dannf]

I think this line: https://github.com/NVIDIA/TensorRT-LLM/blob/28fb9aacaa5a05494635194a9cbb264da9a744bd/cpp/CMakeLists.txt#L473 or https://github.com/NVIDIA/TensorRT-LLM/blob/28fb9aacaa5a05494635194a9cbb264da9a744bd/cpp/CMakeLists.txt#L431 is finding Python3, but pybind11 probably was added before this at https://github.com/NVIDIA/TensorRT-LLM/blob/5bdf997963365ff6ff167e21ab1f53631cf274eb/cpp/CMakeLists.txt#L391, so it found Python. Moving the pybind11 inclusion to after this line would fix it, or pybind11's discovery could be used without re-finding it. As it is, the two discoveries could find different Pythons.

Also it should not be calling python3 before searching for it at https://github.com/NVIDIA/TensorRT-LLM/blob/5bdf997963365ff6ff167e21ab1f53631cf274eb/cpp/CMakeLists.txt#L156.

Thanks @henryiii !