[cmake-developers] [PATCH] Fix FindOpenCL on Mac OS

[Matthäus G. Chajdas]

Hi Jerry,

thanks for giving it a spin :)

I don't have a Mac to test myself - as your changes are confined to
macOS, they look safe to me.

The version is not listed as a required variable, so that's why it
passes through. If you pass in a version into your find_package call, it
should bail out if no version is specified.

I only got one minor nit-pick: Why did you change the line
INTERFACE_INCLUDE_DIRECTORIES "${OpenCL_INCLUDE_DIR}")
to use _DIR instead of _DIRS? I assume this was from testing, not
because that changed something on macOS?

Other than that, the patch looks good to me. I'll apply it this week.

Cheers,
  Matthäus

Am 01.08.2016 um 09:58 schrieb jerry.c.t at web.de:
> Hi,
> 
> I tried the FindOpenCL.cmake with the imported target OpenCL::OpenCL and found out that it does not work on MacOS.
> 
> The first problem is that it does not detect the version. In line 56 the path needs to be changed from ${OpenCL_INCLUDE_DIR}/OpenCL/cl.h" to “${OpenCL_INCLUDE_DIR}/Headers/cl.h“ because OpenCL_INCLUDE_DIR points to the root of the OpenCL framework directory and the Headers are located under Headers.
> 
> Also there seems to be a bug in find_package_handle_standard_args (?). Because without the change above - when there is no version found - the variable OpenCL_VERSION_STRING is empty. But an empty version string does not let cmake fail. This is the output
> 
> -- Looking for CL_VERSION_2_0
> -- Looking for CL_VERSION_2_0 - not found
> -- Looking for CL_VERSION_1_2
> -- Looking for CL_VERSION_1_2 - not found
> -- Looking for CL_VERSION_1_1
> -- Looking for CL_VERSION_1_1 - not found
> -- Looking for CL_VERSION_1_0
> -- Looking for CL_VERSION_1_0 - not found
> -- Found OpenCL: /System/Library/Frameworks/OpenCL.framework (Required is at least version "1.2") 
> 
> The other problems are with the imported locations in line 147ff. With the current solution the linker step fails because OpenCL_LIBRARY points to the root of the framework directory - i.e. /System/Library/Frameworks/OpenCL.framework. Long time ago there was a bug report [1] but the proposed solution was to have an if/else statement to handle the special case of Apple frameworks. So I based my solution on [2] and [3]. This works fine with Makefile and Ninja generators but the Xcode generator still fails. The problem now is that with Xcode 7 Apple switched the way how framework libraries work [5]. The final solution is based on [4] where the Apple framework case is changed to an INTERFACE IMPORTED library and the framework is stored in INTERFACE_LINK_LIBRARIES. This way cmake resolves the library path to -framework OpenCL.
> 
> So with this patch I provide my final solution. Is it correct (it works at least for me now on Win, Linux and Apple)? 
> 
> What’s with this empty version bug? I don’t found a solution for this.
> 
> jerry
> 
> [1] https://cmake.org/Bug/view.php?id=14105
> [2] https://github.com/Kitware/CMake/blob/master/Modules/FindGLUT.cmake
> [3] https://github.com/rpavlik/cmake-modules/blob/master/FindSDL2.cmake
> [4] http://public.kitware.com/pipermail/cmake/2016-April/063179.html
> [5] http://public.kitware.com/pipermail/cmake-developers/2015-August/026110.html
> 
> 
>