Report an error when computecpp_info or compute++ can't be found (#126)

[RossBrunton]

Added a thing to the makefile, let me know if there is a better way of doing this.

[Ruyk]

That looks good actually, thanks!

[DuncanMcBain]

I don't really want to add this, because it will break projects with optional ComputeCpp support (you shouldn't need ComputeCpp to be present if you aren't going to use that). This is handled via the find_package_handle_standard_args* call in the module - though I can see that if it can't find the executable, that is going to fail anyway. I will think about this.

*When you do find_package, if you say REQUIRED, then it will bail when it can't find it - otherwise it just moves on with its life, even when not found.

[Ruyk]

@DuncanMcBain You are right that REQUIRED should handle that, but the error you get if computecpp_info (at least on my experience) is not great, and difficult to track. This would have a clearer error. Maybe there is a better way to get the clear error? I am not fixated on the solution.
Also, if you don't use ComputeCpp in your project, why would you have FindComputeCpp directly on the CMake? wouldn't you have some if /else around it anyway?

[DuncanMcBain]

I guess I'm thinking of scenarios where you have optional support that the user might or might not have - if they provide a ComputeCpp_DIR, you'll use it, otherwise fall back to some (inferior, slower, serial) code. I think it's possible to get an improvement still:

diff --git a/cmake/Modules/FindComputeCpp.cmake b/cmake/Modules/FindComputeCpp.cmake index e265b55..4b15025 100644 --- a/cmake/Modules/FindComputeCpp.cmake +++ b/cmake/Modules/FindComputeCpp.cmake @@ -106,34 +106,38 @@ get_filename_component(computecpp_canonical_root_dir "${ComputeCpp_INCLUDE_DIRS} set(ComputeCpp_ROOT_DIR "${computecpp_canonical_root_dir}" CACHE PATH "The root of the ComputeCpp install") -execute_process(COMMAND ${ComputeCpp_INFO_EXECUTABLE} "--dump-version" - OUTPUT_VARIABLE ComputeCpp_VERSION - RESULT_VARIABLE ComputeCpp_INFO_EXECUTABLE_RESULT OUTPUT_STRIP_TRAILING_WHITESPACE) -if(NOT ComputeCpp_INFO_EXECUTABLE_RESULT EQUAL "0") - message(FATAL_ERROR "Package version - Error obtaining version!") -endif() - -execute_process(COMMAND ${ComputeCpp_INFO_EXECUTABLE} "--dump-is-supported" - OUTPUT_VARIABLE COMPUTECPP_PLATFORM_IS_SUPPORTED - RESULT_VARIABLE ComputeCpp_INFO_EXECUTABLE_RESULT OUTPUT_STRIP_TRAILING_WHITESPACE) -if(NOT ComputeCpp_INFO_EXECUTABLE_RESULT EQUAL "0") - message(FATAL_ERROR "platform - Error checking platform support!") +if(NOT ${ComputeCpp_INFO_EXECUTABLE}) + message(WARNING "Can't find computecpp_info - check ComputeCpp_DIR") + set(COMPUTECPP_DEVICE_COMPILER_FLAGS "") else() - mark_as_advanced(COMPUTECPP_PLATFORM_IS_SUPPORTED) - if (COMPUTECPP_PLATFORM_IS_SUPPORTED) - message(STATUS "platform - your system can support ComputeCpp") - else() - message(WARNING "platform - your system CANNOT support ComputeCpp") + execute_process(COMMAND ${ComputeCpp_INFO_EXECUTABLE} "--dump-version" + OUTPUT_VARIABLE ComputeCpp_VERSION + RESULT_VARIABLE ComputeCpp_INFO_EXECUTABLE_RESULT OUTPUT_STRIP_TRAILING_WHITESPACE) + if(NOT ComputeCpp_INFO_EXECUTABLE_RESULT EQUAL "0") + message(WARNING "Package version - Error obtaining version!") endif() -endif() -execute_process(COMMAND ${ComputeCpp_INFO_EXECUTABLE} - "--dump-device-compiler-flags" - OUTPUT_VARIABLE COMPUTECPP_DEVICE_COMPILER_FLAGS - RESULT_VARIABLE ComputeCpp_INFO_EXECUTABLE_RESULT OUTPUT_STRIP_TRAILING_WHITESPACE) + execute_process(COMMAND ${ComputeCpp_INFO_EXECUTABLE} "--dump-is-supported" + OUTPUT_VARIABLE COMPUTECPP_PLATFORM_IS_SUPPORTED + RESULT_VARIABLE ComputeCpp_INFO_EXECUTABLE_RESULT OUTPUT_STRIP_TRAILING_WHITESPACE) + if(NOT ComputeCpp_INFO_EXECUTABLE_RESULT EQUAL "0") + message(WARNING "platform - Error checking platform support!") + else() + mark_as_advanced(COMPUTECPP_PLATFORM_IS_SUPPORTED) + if (COMPUTECPP_PLATFORM_IS_SUPPORTED) + message(STATUS "platform - your system can support ComputeCpp") + else() + message(WARNING "platform - your system CANNOT support ComputeCpp") + endif() + endif() -if(NOT ComputeCpp_INFO_EXECUTABLE_RESULT EQUAL "0") - message(FATAL_ERROR "compute++ flags - Error obtaining compute++ flags!") + execute_process(COMMAND ${ComputeCpp_INFO_EXECUTABLE} + "--dump-device-compiler-flags" + OUTPUT_VARIABLE COMPUTECPP_DEVICE_COMPILER_FLAGS + RESULT_VARIABLE ComputeCpp_INFO_EXECUTABLE_RESULT OUTPUT_STRIP_TRAILING_WHITESPACE) + if(NOT ComputeCpp_INFO_EXECUTABLE_RESULT EQUAL "0") + message(WARNING "compute++ flags - Error obtaining compute++ flags!") + endif() endif() mark_as_advanced(COMPUTECPP_DEVICE_COMPILER_FLAGS) separate_arguments(COMPUTECPP_DEVICE_COMPILER_FLAGS)

Argh that's a horrible diff - broadly, it elides running the info executable if it can't find it, but will warn you that it's missing. If by some miracle everything else is present in the package, it will still run (but this is something of an advanced use case). Sadly this involves an indentation change which is why diff is large.

[Ruyk]

The slightly better error message you get with @DuncanMcBain approach will suffice for me, so I guess we'll wait to hear from @RossBrunton !