Improvements to the build system #4609

intelfx · 2016-08-20T01:29:29Z

These are various improvements to the CMake buildsystem directed towards "modern" packaging with CMake IMPORTED targets and config-mode find_package().

There are several bugfixes and a slight refactoring of the buildsystem which makes Caffe properly export its transitive dependencies ("usage requirements") to the library users.

Open questions:

most of the dependencies are made PUBLIC (i. e. the client programs will link to them). This mimics the previous behavior, but it might be worth to revise configuration. E. g. the BLAS libraries — do we really need to make clients link to them?

Tests done:

building as a static and shared library for GNU/Linux and Android, linking to a sample project.

…ted caffe target This is the first step towards "modern" IMPORTED-targets-only CMake setup. The find_package modules still need to be rewritten and upstreamed in form of config exports where possible.

Despite Caffe itself does not use OpenMP, explicitly linking to OpenMP should be done when one statically links to a BLAS library which uses OpenMP internally and does not provide proper CMake imported targets with proper dependencies (nobody this so far).

Rationale: these are duplicated in CMakeLists code, and they cannot be removed from there because many definitions need to be exported to the library clients. See issue BVLC#4625.

shelhamer · 2017-02-17T00:36:03Z

Thanks for the build polish.

willyd · 2017-03-17T00:05:13Z

src/caffe/CMakeLists.txt

+                                 PUBLIC
+                                 $<BUILD_INTERFACE:${Caffe_INCLUDE_DIR}>
+                                 $<INSTALL_INTERFACE:include>)
+target_compile_definitions(caffe ${Caffe_DEFINITIONS})


According to the CMake release notes this command was added in 2.8.11. The root CMakeLists currently requires only 2.8.7.

willyd · 2017-03-17T00:06:22Z

src/caffe/CMakeLists.txt

+                                 $<INSTALL_INTERFACE:include>)
+target_compile_definitions(caffe ${Caffe_DEFINITIONS})
+if(Caffe_COMPILE_OPTIONS)
+  target_compile_options(caffe ${Caffe_COMPILE_OPTIONS})


According to the CMake release notes this command was added in 2.8.12. The root CMakeLists currently requires only 2.8.7.

willyd · 2017-03-17T00:07:57Z

@intelfx Looks like we should also bump the minimum CMake version from 2.8.7 to 2.8.12.

severin-lemaignan · 2017-10-17T09:25:55Z

@intelfx While moving toward 'modern' CMake is certainly laudable, IMPORTED targets are not yet very common and I'm sorely missing documentation/example on how to use them. Even the official CMake documentation does not help me much in that regard.

Could you provide examples (possibly in the documentation of CaffeConfig.cmake) showing how to integrate Caffe in a C++ CMake project?

Right now I'm here:

# [...]
find_package(Caffe) 
add_library(Caffe SHARED IMPORTED)
# [...]
add_executable(myapp src/myapp.cpp)
target_link_libraries(myapp Caffe)

This fails because the Caffe includes are not found (I should make clear that Caffe is not installed in /usr/local/ but in a different prefix -- Cmake found it and find_package(Caffe) succeeds).

Adding target_include_directories(replay_with_poses PUBLIC Caffe) before add_executable does not help as the target myapp does not exist yet; adding it after does not help either (headers not found at the add_executable step).

shelhamer added the build label Aug 20, 2016

intelfx force-pushed the BVLC-work-buildsystem branch 2 times, most recently from bc20bb9 to d5e5f8e Compare August 20, 2016 03:06

intelfx mentioned this pull request Aug 24, 2016

#define duplication between CMakeLists and caffe_config.h #4625

Closed

intelfx force-pushed the BVLC-work-buildsystem branch 2 times, most recently from 303e91c to c6d36b6 Compare August 24, 2016 06:49

intelfx mentioned this pull request Aug 24, 2016

#define duplication between CMakeLists and caffe_config.h naibaf7/caffe#41

Closed

intelfx force-pushed the BVLC-work-buildsystem branch from c6d36b6 to b469777 Compare August 25, 2016 01:09

intelfx mentioned this pull request Aug 25, 2016

[RFC] Improvements to the build system naibaf7/caffe#43

Merged

intelfx added 6 commits August 31, 2016 15:20

cmake: fix usage of INCLUDE_DIR/INCLUDE_DIRS in Dependencies.cmake

b9c3c06

cmake/Templates: properly spell OpenCV CMake config file name

a59e647

cmake: refactor deps detection, specify all dependencies in the expor…

ba189d9

…ted caffe target This is the first step towards "modern" IMPORTED-targets-only CMake setup. The find_package modules still need to be rewritten and upstreamed in form of config exports where possible.

net.cpp: do not include test/test_caffe_main.hpp

6200b91

cmake/Templates: remove duplicated #cmakedefines from caffe_config.h.in

6ed799c

Rationale: these are duplicated in CMakeLists code, and they cannot be removed from there because many definitions need to be exported to the library clients. See issue BVLC#4625.

intelfx force-pushed the BVLC-work-buildsystem branch from b469777 to 6ed799c Compare August 31, 2016 12:21

shelhamer merged commit 9bb1b9c into BVLC:master Feb 17, 2017

naibaf7 mentioned this pull request Mar 12, 2017

Remove Makefile.config build completely. #5391

Closed

willyd reviewed Mar 17, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements to the build system #4609

Improvements to the build system #4609

intelfx commented Aug 20, 2016

shelhamer commented Feb 17, 2017

willyd Mar 17, 2017 •

edited

Loading

willyd Mar 17, 2017

willyd commented Mar 17, 2017

severin-lemaignan commented Oct 17, 2017 •

edited

Loading

Improvements to the build system #4609

Improvements to the build system #4609

Conversation

intelfx commented Aug 20, 2016

shelhamer commented Feb 17, 2017

willyd Mar 17, 2017 • edited Loading

Choose a reason for hiding this comment

willyd Mar 17, 2017

Choose a reason for hiding this comment

willyd commented Mar 17, 2017

severin-lemaignan commented Oct 17, 2017 • edited Loading

willyd Mar 17, 2017 •

edited

Loading

severin-lemaignan commented Oct 17, 2017 •

edited

Loading