Skip to content

Commit

Permalink
cpp: Require CXX_STANDARD 11 - fixes #578
Browse files Browse the repository at this point in the history 
Signed-off-by: Benn Snyder <benn.snyder@gmail.com>
  • Loading branch information
@piedar
piedar committed Feb 24, 2019
1 parent 52908d4 commit 3516072
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
8 changes: 4 additions & 4 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Notice: If you have the newer Kinect v2 (XBox One), use [OpenKinect/libfreenect2
To build libfreenect, you'll need

- [libusb](http://libusb.info) >= 1.0.18
- [CMake](http://cmake.org) >= 2.8.12
- [CMake](http://cmake.org) >= 3.1.0
- [python](http://python.org) >= 2.7 or >= 3.3 (only if BUILD_PYTHON=ON or BUILD_PYTHON2=ON or BUILD_PYTHON3=ON or BUILD_REDIST_PACKAGE=OFF)

For the examples, you'll need
Expand All @@ -44,12 +44,12 @@ For example, to build the Python wrapper (defaults to system Python):

cmake .. -DBUILD_PYTHON=ON
make

Or the Python 3 wrapper:

cmake .. -DBUILD_PYTHON3=ON
make


You can specify a build with debug symbols:

Expand Down
5 changes: 4 additions & 1 deletion wrappers/cpp/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
cmake_minimum_required(VERSION 3.1.0)
set_property(DIRECTORY PROPERTY CXX_STANDARD 11)

install(FILES libfreenect.hpp
DESTINATION ${PROJECT_INCLUDE_INSTALL_DIR})

Expand All @@ -19,4 +22,4 @@ if (BUILD_EXAMPLES)
install(TARGETS freenect-cppview freenect-cpp_pcview
DESTINATION bin)
endif()
ENDIF()
endif()

4 comments on commit 3516072

@ubunux
Copy link

@ubunux ubunux commented on 3516072 Mar 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
I use cmake-3.16.5-Linux-x86_64 to build libfreenect and it fails, logs like below:

In file included from /mnt/code/OSP/libfreenect/wrappers/cpp/cpp_pc_view.cpp:30:0:
/mnt/code/OSP/libfreenect/wrappers/cpp/./libfreenect.hpp:182:8: error: ‘unique_ptr’ in namespace ‘std’ does not name a template type
   std::unique_ptr<uint8_t[]> m_rgb_buffer;
        ^
/mnt/code/OSP/libfreenect/wrappers/cpp/./libfreenect.hpp: In member function ‘void Freenect::FreenectDevice::setVideoFormat(freenect_video_format, freenect_resolution)’:
/mnt/code/OSP/libfreenect/wrappers/cpp/./libfreenect.hpp:115:5: error: ‘m_rgb_buffer’ was not declared in this scope
     m_rgb_buffer.reset(new uint8_t[mode.bytes]);
     ^

However, build can pass by add this tag, is this cmake bug?

diff --git a/wrappers/cpp/CMakeLists.txt b/wrappers/cpp/CMakeLists.txt
index 6a47d12..10afe25 100644
--- a/wrappers/cpp/CMakeLists.txt
+++ b/wrappers/cpp/CMakeLists.txt
@@ -1,5 +1,5 @@
 cmake_minimum_required(VERSION 3.1.0)
-set_property(DIRECTORY PROPERTY CXX_STANDARD 11)
+set(CMAKE_CXX_STANDARD 11)
 
 install(FILES libfreenect.hpp
         DESTINATION ${PROJECT_INCLUDE_INSTALL_DIR})

@piedar
Copy link
Contributor Author

@piedar piedar commented on 3516072 Mar 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CMAKE_CXX_STANDARD is supposed to just set the default value for CXX_STANDARD so it should be the same. Maybe the DIRECTORY part is not working. I think the right way to do this now is to add target_compile_features(mylib PUBLIC cxx_std_11) to each target.

@ubunux
Copy link

@ubunux ubunux commented on 3516072 Mar 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for your reply!
Yes, you are right.
Refer to documents from cmake, CXX_STANDARD not including in Properties on Directories.
https://cmake.org/cmake/help/v3.16/manual/cmake-properties.7.html#directory-properties

@piedar
Copy link
Contributor Author

@piedar piedar commented on 3516072 Mar 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, that makes sense - not every property can apply to the directory. Hopefully #614 will fix it for you.

Please sign in to comment.