Skip to content

Conversation

@ahundt
Copy link

@ahundt ahundt commented Feb 25, 2016

No description provided.

@kohrt
Copy link

kohrt commented Feb 26, 2016

Thanks for the PR. I just have some questions.
Why does OS X needs that header, what is wrong with the one provided by OS X?
And why did you add CXX to the project(...), isn't it enabled by default anyway?

@ahundt
Copy link
Author

ahundt commented Feb 28, 2016

regarding CXX I think it is just recommended by CMake in their docs. I'm also having some issues where the flags set break clang compilation but for now I'm just using a hardcoded -std=c++11 flag. The header simply doesn't seem to be present on OS X, and this is the exact header found in libfreenect2 for the same reason. I guess since this projects uses libfreenect2 it may be possible to find the one they defined. I didn't think of that possibility for some reason when I first made the change, perhaps they should install it so dependents can use it? I just checked and they currently do not.

@xlz
Copy link
Contributor

xlz commented Feb 28, 2016

I think OP meant including cl.hpp. libfreenect2 includes one copy of cl.hpp internally. Part of the reason is System integrity protection on Mac OS X which prevent root from doing things OpenKinect/libfreenect2#430

@ahundt
Copy link
Author

ahundt commented Feb 28, 2016

Yeah that's what I meant

@ahundt
Copy link
Author

ahundt commented Feb 28, 2016

Per @xlz in OpenKinect/libfreenect2#595 they will not be installing cl.hpp, so it makes sense to include it here.

@ahundt ahundt changed the title CMake build fixes for OS X for CL.h based on fixes in libfreenect2 CMake build fixes for OS X for CL.hpp based on fixes in libfreenect2 Feb 28, 2016
@kohrt kohrt merged commit 87a7f4a into code-iai:master Feb 29, 2016
@kohrt
Copy link

kohrt commented Feb 29, 2016

merged the PR. Thanks for detailed description of the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants