orbisvicis
commented
10 years ago Open orbisvicis opened 10 years ago
orbisvicis
commented
10 years ago orbisvicis
commented
10 years ago @sthalik: this will affect headtracker's build.
sthalik
commented
10 years ago In the diff posted, can't find a cache variable declaration, that would control static/shared build type.
Is shared object version information really necessary, given how the api's relatively stable?
Thanks for the notice about headtracker breakage, in any case.
Overall, don't see how the patch improves on things, with the exception of PIC enablement in a generic fashion, but final say of course goes to @uricamic !
orbisvicis
commented
10 years ago I'm not sure what you mean, BUILD_SHARED_LIBS is a builtin CMake variable that defaults to FALSE (build static libs). It is a cache variable in the sense that it is saved in CMakeCache.txt
Irrespective of the api, which I haven't really examined, soversion is also useful to specify version compatibility between forks of flandmark.
I like the changes because they simplify CMakeLists.txt as well as building on linux (I've uploaded an Arch Linux package @ https://aur.archlinux.org/packages/flandmark-git/): * control which version the examples link against * name-scheme (shared name) common (not ubiqitous) on linux * name-scheme incompatible on windows (static/shared have same extension) but not a problem because only one version built
But of course final say and etc... !
uricamic
commented
10 years ago @orbisvicis: thanks for the patch, I tried it on the new universal version of flandmark (not publicly available yet) and I really like how it simplified several things. However, there are some things:
I am now busy with the new project, but I will probably incorporate this patch here soon.
Anyways, thanks for the patch, if not here, it will be definitely useful in the upcoming projec.
orbisvicis
commented
10 years ago The following patch addresses the second point, as for the first I'm not really sure if you want to bump the minimum required CMake version or support both approaches or only the former:
From 9432735689a47f97da98924b0705816a5e147f4e Mon Sep 17 00:00:00 2001
From: Yclept Nemo <-------------------->
Date: Thu, 23 Jan 2014 10:39:16 -0500
Subject: [PATCH] CMake: change variables shown in configuration GUI
hide: CMAKE_INSTALL_PREFIX
show: BUILD_SHARED_LIBS (default TRUE), CMAKE_SKIP_BUILD_RPATH
(default FALSE)
---
CMakeLists.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 0e55a45..4637deb 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -6,6 +6,11 @@ project(flandmark)
set(flandmark_VERSION_MAJOR 1)
set(flandmark_VERSION_MINOR 7)
+# configure which options are shown in the gui
+MARK_AS_ADVANCED(CMAKE_INSTALL_PREFIX)
+SET(BUILD_SHARED_LIBS "TRUE" CACHE BOOL "Global flag to cause add_library to create shared libraries if on.")
+SET(CMAKE_SKIP_BUILD_RPATH "FALSE" CACHE BOOL "Do not include RPATHs in the build tree.")
+
if(NOT CMAKE_BUILD_TYPE)
set(CMAKE_BUILD_TYPE RELEASE CACHE STRING
"Choose the type of build, options are: None Debug Release RelWithDebInfo MinSizeRel."
--
1.8.2.2
More changes:
From 300c02a97728d3c74711cd001da1d294288e6be6 Mon Sep 17 00:00:00 2001
From: Yclept Nemo <-------------------->
Date: Sat, 25 Jan 2014 10:28:06 -0500
Subject: [PATCH 1/2] Avoid conflict between shared/static libs in MSVC
Prepend the "lib" prefix when building static libraries. This only
affects MSVC, all other platforms already default to the "lib"
prefix.
By default import libraries and static libraries share the same name
in MSVC. This patch avoids name conflicts between shared and static
libraries built in the same tree using MSVC:
shared: flandmark.dll
import: flandmark.lib
static: libflandmark.lib
---
libflandmark/CMakeLists.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libflandmark/CMakeLists.txt b/libflandmark/CMakeLists.txt
index e707f26..ddfa0e1 100644
--- a/libflandmark/CMakeLists.txt
+++ b/libflandmark/CMakeLists.txt
@@ -2,6 +2,9 @@ find_package( OpenCV REQUIRED )
include_directories(${OpenCV_INCLUDE_DIRS})
add_library(flandmark flandmark_detector.cpp flandmark_detector.h liblbp.cpp liblbp.h)
+if(NOT ${BUILD_SHARED_LIBS})
+ set_target_properties(flandmark PROPERTIES PREFIX "lib")
+endif()
target_link_libraries(flandmark ${OpenCV_LIBS})
set_target_properties(flandmark PROPERTIES POSITION_INDEPENDENT_CODE TRUE)
set_target_properties(flandmark PROPERTIES
--
1.8.2.2
Now that there are no naming conflicts, the following expects CMake output target names to be honored in order to work. @sthalik: you should use FindFLANDMARK.cmake in headtracker's CMakeLists.cmake if you want to be able to specify static/shared versions of flandmark, rather than flandmarkConfig.cmake which will only reference the last version built (ie assume flandmarkConfig.cmake contains the packager's prefered version to link against).
From 2a0ea0178025232eb544e942be108196e792cae0 Mon Sep 17 00:00:00 2001
From: Yclept Nemo <-------------------->
Date: Sat, 25 Jan 2014 12:29:45 -0500
Subject: [PATCH 2/2] Add a CMake find module for flandmark
---
libflandmark/FindFLANDMARK.cmake | 60 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 libflandmark/FindFLANDMARK.cmake
diff --git a/libflandmark/FindFLANDMARK.cmake b/libflandmark/FindFLANDMARK.cmake
new file mode 100644
index 0000000..507fa3c
--- /dev/null
+++ b/libflandmark/FindFLANDMARK.cmake
@@ -0,0 +1,60 @@
+# - Try to find the flandmark facial landmark detector library
+#
+# =============================================================================
+# Once done this will define:
+#
+# FLANDMARK_FOUND TRUE if found; FALSE otherwise
+# FLANDMARK_INCLUDE_DIRS where to find flandmark_detector.h
+# FLANDMARK_LIBRARIES the libraries to link against
+#
+# =============================================================================
+# Variables used by this module:
+#
+# FLANDMARK_PREFER_STATIC If TRUE and available, link against the static
+# flandmark library. Otherwise select the shared
+# version
+#
+# =============================================================================
+# To use this from another project:
+#
+# create a directory named cmake/Modules under the project root, copy this file
+# (FindFLANDMARK.cmake) there, and in the top-level CMakeLists.txt include:
+# set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH}
+# "${CMAKE_SOURCE_DIR}/cmake/Modules/")
+#
+# =============================================================================
+
+find_path(FLANDMARK_INCLUDE_DIR flandmark_detector.h)
+
+set(CMAKE_FIND_LIBRARY_SUFFIXES ".so" ".dll")
+find_library(FLANDMARK_LIBRARY_STATIC NAMES flandmark)
+
+set(CMAKE_FIND_LIBRARY_PREFIXES "lib")
+set(CMAKE_FIND_LIBRARY_SUFFIXES ".a" ".lib")
+find_library(FLANDMARK_LIBRARY_SHARED NAMES flandmark)
+
+if(FLANDMARK_LIBRARY_STATIC and FLANDMARK_LIBRARY_SHARED)
+ if(FLANDMARK_PREFER_STATIC)
+ set(FLANDMARK_LIBRARY ${FLANDMARK_LIBRARY_STATIC})
+ else()
+ set(FLANDMARK_LIBRARY ${FLANDMARK_LIBRARY_SHARED})
+ endif()
+elseif(FLANDMARK_LIBRARY_STATIC)
+ set(FLANDMARK_LIBRARY ${FLANDMARK_LIBRARY_STATIC})
+elseif(FLANDMARK_LIBRARY_SHARED)
+ set(FLANDMARK_LIBRARY ${FLANDMARK_LIBRARY_SHARED})
+else()
+ set(FLANDMARK_LIBRARY "FLANDMARK_LIBRARY-NOTFOUND")
+endif()
+
+set(FLANDMARK_LIBRARIES ${FLANDMARK_LIBRARY})
+set(FLANDMARK_INCLUDE_DIRS ${FLANDMARK_INCLUDE_DIR})
+
+include(FindPackageHandleStandardArgs)
+# handle the QUIETLY and REQUIRED arguments and set FLANDMARK_FOUND to TRUE
+# if all listed variables are TRUE
+find_package_handle_standard_args(FLANDMARK
+ DEFAULT_MSG
+ FLANDMARK_LIBRARY FLANDMARK_INCLUDE_DIR)
+
+mark_as_advanced(FLANDMARK_INCLUDE_DIR FLANDMARK_LIBRARY)
--
1.8.2.2
In addition to simplifying flandmarkConfig.cmake, provide absolute paths like find_library():
From 39976612539a8e82b7179aeee661c94369578e13 Mon Sep 17 00:00:00 2001
From: Yclept Nemo <-------------------->
Date: Sat, 25 Jan 2014 14:18:23 -0500
Subject: [PATCH 1/2] Simplify flandmarkConfig.cmake.in
---
libflandmark/CMakeLists.txt | 4 ++--
libflandmark/flandmarkConfig.cmake.in | 12 +++---------
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/libflandmark/CMakeLists.txt b/libflandmark/CMakeLists.txt
index ddfa0e1..671dc6f 100644
--- a/libflandmark/CMakeLists.txt
+++ b/libflandmark/CMakeLists.txt
@@ -13,8 +13,8 @@ set_target_properties(flandmark PROPERTIES
)
#setup Config.cmake
-SET(FLANDMARK_BASE_DIR "${PROJECT_SOURCE_DIR}/libflandmark")
-set(FLANDMARK_BINARY_DIR "${PROJECT_BINARY_DIR}/libflandmark")
+SET(FLANDMARK_INCLUDE_DIRS "${PROJECT_SOURCE_DIR}/libflandmark")
+get_property(FLANDMARK_LIBRARIES TARGET flandmark PROPERTY LOCATION)
configure_file(flandmarkConfig.cmake.in
"${PROJECT_BINARY_DIR}/libflandmark/flandmarkConfig.cmake" @ONLY
)
diff --git a/libflandmark/flandmarkConfig.cmake.in b/libflandmark/flandmarkConfig.cmake.in
index e0fad9b..cd4f95e 100644
--- a/libflandmark/flandmarkConfig.cmake.in
+++ b/libflandmark/flandmarkConfig.cmake.in
@@ -1,11 +1,5 @@
-SET(FLANDMARK_BASE_DIR "@FLANDMARK_BASE_DIR@")
-SET(FLANDMARK_BINARY_DIR "@FLANDMARK_BINARY_DIR@")
+SET(FLANDMARK_INCLUDE_DIRS "@FLANDMARK_INCLUDE_DIRS@")
-SET(FLANDMARK_INCLUDE_DIRS ${FLANDMARK_BASE_DIR})
-
-SET(FLANDMARK_LINK_DIRS ${FLANDMARK_BINARY_DIR} ${FLANDMARK_BINARY_DIR}/Release)
-
-SET(FLANDMARK_LIBRARIES flandmark_shared)
-
-SET(FLANDMARK_FOUND 1)
+SET(FLANDMARK_LIBRARIES "@FLANDMARK_LIBRARIES@")
+SET(FLANDMARK_FOUND "TRUE")
--
1.8.2.2
...
From 2ab7cd8d656022152b01f73b7a0fa4fc8fc5d3bd Mon Sep 17 00:00:00 2001
From: Yclept Nemo <-------------------->
Date: Sat, 25 Jan 2014 14:24:36 -0500
Subject: [PATCH 2/2] Update example external CMakeLists.txt
track previous commit 39976612539a8e82b7179aeee661c94369578e13
---
examples/external/CMakeLists.txt | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/examples/external/CMakeLists.txt b/examples/external/CMakeLists.txt
index 36ae912..7e84bc5 100644
--- a/examples/external/CMakeLists.txt
+++ b/examples/external/CMakeLists.txt
@@ -41,7 +41,7 @@ SET(CMAKE_VERBOSE_MAKEFILE ON)
SET(CMAKE_BUILD_TYPE "Debug")
SET(CMAKE_CXX_FLAGS "-D_ENGLISH")
-######## INCLUDE AND LINK DIRECTORIES ########
+######## INCLUDE DIRECTORIES ########
INCLUDE_DIRECTORIES(
/usr/local/include
@@ -51,11 +51,6 @@ INCLUDE_DIRECTORIES(
${FLANDMARK_INCLUDE_DIRS}
)
-LINK_DIRECTORIES(
- ${FLANDMARK_LINK_DIRS}
- ${OpenCV_LIB_DIR}
-)
-
######## MAIN EXECUTABLES ########
--
1.8.2.2
Ok, that's that. I don't expect to post any more patches. FTR, I haven't tested any of these on windows.
orbisvicis
commented
10 years ago Oops.
From e286e8e3d7b9e3d2d3b443bbf1ef156d8c84c985 Mon Sep 17 00:00:00 2001
From: Yclept Nemo <-------------------->
Date: Sat, 25 Jan 2014 15:43:36 -0500
Subject: [PATCH] Require CMake >= 2.8 to avoid CMP0012 warnings
---
CMakeLists.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4637deb..789e537 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 2.6)
+cmake_minimum_required(VERSION 2.8)
project(flandmark)
--
1.8.2.2
Could we at the very least not rename the flandmark_static library? This violates POLA.
@uricamic what is status on this issue?