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

2016-08-07 Thread Matthäus G . Chajdas
Here we go:

https://cmake.org/gitweb?p=stage/cmake.git;a=commit;h=48dc6343bba3b3f296d35ab060681c50f0eb8cde

Thanks again for the patch!

Cheers,
  Matthäus

Am 05.08.2016 um 22:09 schrieb Matthäus G. Chajdas:
> Hi,
> 
> all right. Yes, that sounds like a problem with
> find_package_handle_standard_args, I wonder if setting the version to
> 0.0 by default would solve that particular problem?
> 
> _DIR and _DIRS is there because that seems to be true for most packages.
> 
> I'll push your patch this weekend - thanks again.
> 
> Cheers,
>   Matthäus
> 
> Am 01.08.2016 um 21:04 schrieb jerry@web.de:
>> Hi,
>>
>>> 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.
>> Sadly not. The current version in master does not fail when invoked with 
>> "find_package(OpenCL 1.2 REQUIRED)". My first email shows exactly the 
>> output. You see that it says "Required is at least version "1.2""
>> while also saying "Found OpenCL:..." while also no version was found. The
>> patch now fails if invoked for example with find_package(OpenCL 2.0 
>> REQUIRED).
>> It seems that  does not work as expected when
>> OpenCL_VERSION_STRING is empty.
>>
>>> I assume this was from testing, not
>>> because that changed something on macOS?
>> You are right _DIR and _DIRS are working.
>> I think I only changed it because for example FindGLUT uses it, for the 
>> library the singular variant LIBRARY is used and I
>> don't understand the difference of _DIR and _DIRS :)
>>
>> Jerry 
>>
>> On 01.08.2016 14:32, Matthäus G. Chajdas wrote:
>>> 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
>>>
> 

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

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

2016-08-05 Thread Matthäus G . Chajdas
Hi,

all right. Yes, that sounds like a problem with
find_package_handle_standard_args, I wonder if setting the version to
0.0 by default would solve that particular problem?

_DIR and _DIRS is there because that seems to be true for most packages.

I'll push your patch this weekend - thanks again.

Cheers,
  Matthäus

Am 01.08.2016 um 21:04 schrieb jerry@web.de:
> Hi,
> 
>> 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.
> Sadly not. The current version in master does not fail when invoked with 
> "find_package(OpenCL 1.2 REQUIRED)". My first email shows exactly the 
> output. You see that it says "Required is at least version "1.2""
> while also saying "Found OpenCL:..." while also no version was found. The
> patch now fails if invoked for example with find_package(OpenCL 2.0 REQUIRED).
> It seems that  does not work as expected when
> OpenCL_VERSION_STRING is empty.
> 
>> I assume this was from testing, not
>> because that changed something on macOS?
> You are right _DIR and _DIRS are working.
> I think I only changed it because for example FindGLUT uses it, for the 
> library the singular variant LIBRARY is used and I
> don't understand the difference of _DIR and _DIRS :)
> 
> Jerry 
> 
> On 01.08.2016 14:32, Matthäus G. Chajdas wrote:
>> 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
>>

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

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

2016-08-01 Thread jerry . c . t
Hi,

> 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.
Sadly not. The current version in master does not fail when invoked with 
"find_package(OpenCL 1.2 REQUIRED)". My first email shows exactly the 
output. You see that it says "Required is at least version "1.2""
while also saying "Found OpenCL:..." while also no version was found. The
patch now fails if invoked for example with find_package(OpenCL 2.0 REQUIRED).
It seems that find_package_handle_standard does not work as expected when
OpenCL_VERSION_STRING is empty.

> I assume this was from testing, not
> because that changed something on macOS?
You are right _DIR and _DIRS are working.
I think I only changed it because for example FindGLUT uses it, for the 
library the singular variant LIBRARY is used and I
don't understand the difference of _DIR and _DIRS :)

Jerry 

On 01.08.2016 14:32, Matthäus G. Chajdas wrote:
> 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
> 
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

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

2016-08-01 Thread 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@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
> 
> 
> 

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers