Merge lp:~jpakkane/unity-scopes-api/randomstuff into lp:unity-scopes-api

[Jussi Pakkanen]

Various fixes.

Bunch of random stuff I found.

[Michi Henning (michihenning)]

Thanks for the review!

One minor glitch:

link_directories(${UNITY_API_LIBRARY_DIRS})

I think that line should stay in. That's because it adds the location of libunit-api to the rpath when linking the binaries. Without it, the unity-api lib directory is not in the rpath which means, when I run the tests with a libunity-api that's somewhere in my staging area, and I've set PKG_CONFIG_PATH to point at the install location, the tests fail because they can't locate libunity-api.so at run time.

I'd prefer not to have to set LD_LIBRARY_PATH separately to make it work.

review: Needs Fixing

[Jussi Pakkanen (jpakkane)]

I can see why that would be useful. However:

- CMake documentation recommends against using link_directories

- for consistency we should do this for every dependency

- Debian packagers discourage the use of rpath

- if you want to add stuff to rpath, it should be done explicitly with set_target_properties

- when you have the .so both in a system directory and a custom dir, LD_LIBRARY_PATH is the only sane way to select which one is chosen (and which can be changed without recompiling)

- the simple way of solving this is to have a daily build PPA of unity-api that you can use, which is the way most projects do it

If you still feel it should be there, I can add it back in.

[Michi Henning (michihenning)]

OK, you've convinced me. I'll live with setting LD_LIBRARY_PATH :-)

review: Approve

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2013-05-29 08:51:07 +0000
+++ CMakeLists.txt	2013-05-29 14:36:31 +0000
@@ -63,8 +63,7 @@
 find_package(Boost 1.49.0 COMPONENTS regex REQUIRED)
 pkg_check_modules(UNITY_API libunity-api REQUIRED)
 set(OTHER_INCLUDE_DIRS ${OTHER_INCLUDE_DIRS} ${UNITY_API_INCLUDE_DIRS})
-set(OTHER_LIBS ${OTHER_LIBS} ${UNITY_API_LIBRARIES})
-link_directories(${UNITY_API_LIBRARY_DIRS})
+set(OTHER_LIBS ${OTHER_LIBS} ${UNITY_API_LDFLAGS})
 
 # Standard install paths
 include(GNUInstallDirs)
@@ -113,7 +112,7 @@
 set(LIBS ${UNITY_SCOPES_LIB} ${OTHER_LIBS})
 
 # Library install prefix
-set(LIB_INSTALL_PREFIX lib/${CMAKE_LIBRARY_ARCHITECTURE})
+set(LIB_INSTALL_PREFIX "lib" CACHE PATH "Destination install dir for the library")
 
 set(LIBDIR ${CMAKE_INSTALL_PREFIX}/${LIB_INSTALL_PREFIX} CACHE PATH "Destination install dir for the library")
 set(libdir ${LIBDIR})

=== modified file 'test/CMakeLists.txt'
--- test/CMakeLists.txt	2013-05-29 04:57:50 +0000
+++ test/CMakeLists.txt	2013-05-29 14:36:31 +0000
@@ -1,3 +1,7 @@
+set(TEST_BUILD_ROOT ${CMAKE_CURRENT_BINARY_DIR})
+configure_file(scope-api-testconfig.h.in scope-api-testconfig.h @ONLY)
+include_directories(${CMAKE_CURRENT_BINARY_DIR})
+
 add_subdirectory(gtest)
 add_subdirectory(headers)
 add_subdirectory(copyright)

=== modified file 'test/gtest/unity/api/scopes/ScopeBase/CMakeLists.txt'
--- test/gtest/unity/api/scopes/ScopeBase/CMakeLists.txt	2013-05-29 04:57:50 +0000
+++ test/gtest/unity/api/scopes/ScopeBase/CMakeLists.txt	2013-05-29 14:36:31 +0000
@@ -1,6 +1,3 @@
-# This allows us to use a shared library in the current dir for testing without having to set LD_LIBRARY_PATH
-add_definitions(-DTESTLIBDIR=\"${CMAKE_CURRENT_BINARY_DIR}\")
-
 add_executable(ScopeBase_test ScopeBase_test.cpp)
 target_link_libraries(ScopeBase_test ${LIBS} ${TESTLIBS})
 

=== modified file 'test/gtest/unity/api/scopes/ScopeBase/ScopeBase_test.cpp'
--- test/gtest/unity/api/scopes/ScopeBase/ScopeBase_test.cpp	2013-05-29 04:57:50 +0000
+++ test/gtest/unity/api/scopes/ScopeBase/ScopeBase_test.cpp	2013-05-29 14:36:31 +0000
@@ -19,7 +19,7 @@
 #include <unity/api/scopes/internal/ScopeBaseImpl.h>
 #include <unity/api/scopes/internal/DynamicLoader.h>
 #include <unity/UnityExceptions.h>
-
+#include <scope-api-testconfig.h>
 #include <gtest/gtest.h>
 
 using namespace unity::api::scopes;
@@ -27,7 +27,7 @@
 
 namespace
 {
-    char const* scopelib = TESTLIBDIR "/libscopelib.so";
+    char const* scopelib = TEST_BUILD_ROOT "/gtest/unity/api/scopes/ScopeBase/libscopelib.so";
 }
 
 bool create_called = false;

=== modified file 'test/gtest/unity/api/scopes/internal/DynamicLoader/CMakeLists.txt'
--- test/gtest/unity/api/scopes/internal/DynamicLoader/CMakeLists.txt	2013-05-29 04:57:50 +0000
+++ test/gtest/unity/api/scopes/internal/DynamicLoader/CMakeLists.txt	2013-05-29 14:36:31 +0000
@@ -1,6 +1,3 @@
-# This allows us to use a shared library in the current dir for testing without having to set LD_LIBRARY_PATH
-add_definitions(-DTESTLIBDIR=\"${CMAKE_CURRENT_BINARY_DIR}\")
-
 add_executable(DynamicLoader_test DynamicLoader_test.cpp)
 target_link_libraries(DynamicLoader_test ${LIBS} ${TESTLIBS})
 

=== modified file 'test/gtest/unity/api/scopes/internal/DynamicLoader/DynamicLoader_test.cpp'
--- test/gtest/unity/api/scopes/internal/DynamicLoader/DynamicLoader_test.cpp	2013-05-29 04:57:50 +0000
+++ test/gtest/unity/api/scopes/internal/DynamicLoader/DynamicLoader_test.cpp	2013-05-29 14:36:31 +0000
@@ -20,15 +20,16 @@
 #include <unity/UnityExceptions.h>
 
 #include <gtest/gtest.h>
-#include <boost/regex.hpp>  // BUGFIX: We don't use std::regex because, as of gcc 4.7.2, it is still broken
+#include <boost/regex.hpp>  // Use Boost implementation until http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53631 is fixed.
+#include <scope-api-testconfig.h>
 
 using namespace std;
 using namespace unity::api::scopes::internal;
 
 namespace
 {
-    char const* goodlib = TESTLIBDIR "/libtestlib.so";
-    char const* badlib = TESTLIBDIR "/libbadtestlib.so";
+    char const* goodlib = TEST_BUILD_ROOT "/gtest/unity/api/scopes/internal/DynamicLoader/libtestlib.so";
+    char const* badlib = TEST_BUILD_ROOT "/gtest/unity/api/scopes/internal/DynamicLoader/libbadtestlib.so";
 }
 
 // Basic test.

=== added file 'test/scope-api-testconfig.h.in'
--- test/scope-api-testconfig.h.in	1970-01-01 00:00:00 +0000
+++ test/scope-api-testconfig.h.in	2013-05-29 14:36:31 +0000
@@ -0,0 +1,8 @@
+/* This file contains all configuration data needed to run unit tests. */
+
+#ifndef SCOPE_API_TESTCONFIG
+#define SCOPE_API_TESTCONFIG
+
+#define TEST_BUILD_ROOT "@TEST_BUILD_ROOT@"
+
+#endif
\ No newline at end of file