Pinocchio: Disable python interface if python is not found

[ahoarau]

Right now BUILD_PYTHON_INTERFACE is ON by default (fine), but if python is not found it throws an error instead of just sending a big warning and disabling the python interface.

https://github.com/stack-of-tasks/pinocchio/blob/master/CMakeLists.txt#L132

[jcarpent]

This was made on purpose. But, indeed, your option is valid too.

[ahoarau]

That's what it is doing for doxygen. If not found -> no docs and big warning.
To me that is easier to understand than not building at all and looking for the error.

[traversaro]

if python is not found it throws an error instead of just sending a big warning and disabling the python interface.

Personal opinion: if an option is set and the relative dependency is not found, the build should clearly fail, otherwise it is a nightmare for packagers (see https://www.youtube.com/watch?v=sBP17HQAQjk&feature=youtu.be&t=653 for an extended rationale). A compromise I usually make for convenience of people compiling libraries manually from source is to auto-detect the default value of a given option, but in case the user specifies explicitly the option, it will override the default value and throw an error if the requested dependency is not available (see https://github.com/robotology/idyntree/blob/master/cmake/iDynTreeDependencies.cmake#L18). However, this is still quite an additional complexity that may not be worth to include in any project.

[ahoarau]

Definitely a good point here. I'm still in favor to the "added complexity for the package maintainer", not the user. The compromise you describe is very fine with me:

cmake .. (no option, default build python is ON) --> python not found --> Warning + turn OFF the python interface
cmake .. (no option, default build python is ON) --> python found --> All green, turn ON the python interface
cmake .. -DBUILD_PYTHON_INTERFACE=OFF --> not searching python --> All green, turn OFF the python interface
cmake .. -DBUILD_PYTHON_INTERFACE=ON , and python not found --> Fatal ERROR

As soon as it is written somewhere, it simple for everyone.

[nim65s]

@ahoarau : I like your API proposal, but this is not currently possible with CMake option(), so this would require additional complexity in our CMake codebase.

[proyan]

cmake .. (no option, default build python is ON) --> python not found --> Warning + turn OFF the python interface

How do you differentiate between default python build=ON and user-defined python build=ON?
I suppose that we would be adding a set of default option variables, and a copy of the same variables that the user can set?

I agree with @traversaro here, I think this is might be more complex than needed.

[cmastalli]

Yes, I also agree with @proyan and @traversaro.
It is perfectly acceptable and expected to throw an error.

For me the solution is to improve the reported message by Cmake, i.e. letting the users know that they could disable the Python bindings.

[ahoarau]

Very much in favor of adding a very explicit message and keeping the build failing.

[jcarpent]

Solved by #1328.

[traversaro]

I agree with @traversaro here, I think this is might be more complex than needed.

Just to clarify so we are aligned, the complexity we are talking about here is:
~cmake
find_package(Dependency QUIET)
option(BUILD_WITH_DEPENDENCY "Build optional stuff that depends on Dependency" ${Dependency_FOUND})
if(BUILD_WITH_DEPENDENCY)
find_package(Dependency REQUIRED)
endif()
~

[jcarpent]

I see. You mean, this implementation would be aligned with the request?

[traversaro]

I see. You mean, this implementation would be aligned with the request?

I think it depends on which request we were discussing about, if https://github.com/stack-of-tasks/pinocchio/issues/1323#issuecomment-709206979 it is slightly different because it just use the value of whether python is found or not depending only the first time the project is configured. However, it is still additional complexity, so I do not think it changes the discussion, I just wanted to leave this snippet as a reference if someone will be reading the discussion in the future.