Merge lp:~michihenning/unity-api/static-test-lib into lp:unity-api

[Michi Henning]

This changes the build environment to link the tests against a static version of the library, allows us to write unit tests for internal classes that are not visible through the public API (support classes in the internal namespace that don't have a public pimple facade).

Code is always compiled with -fPIC, so it can be used for both static and dynamic versions and we don't need to compile every source file twice with different compiler flags.

This changes the build environment to link the tests against a static version of the library, allows us to write unit tests for internal classes that are not visible through the public API (support classes in the internal namespace that don't have a public pimple facade).

Code is always compiled with -fPIC, so it can be used for both static and dynamic versions and we don't need to compile every source file twice with different compiler flags.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:61
http://jenkins.qa.ubuntu.com/job/unity-api-ci/78/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-saucy-armhf-ci/35
        deb: http://jenkins.qa.ubuntu.com/job/unity-api-saucy-armhf-ci/35/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-saucy-i386-ci/36

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-api-ci/78/rebuild

review: Approve (continuous-integration)

[Jussi Pakkanen (jpakkane)]

This is the kind of code that makes you want to take long showers to get the dirt off but I can't really think of a cleaner way to do it.

One potential change is that instead of using a global argument:

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fPIC")

you should consider using a per-target compile flag to the library (i.e. with set_properties).

review: Needs Fixing

[Michi Henning (michihenning)]

> This is the kind of code that makes you want to take long showers to get the
> dirt off but I can't really think of a cleaner way to do it.

Not entirely sure what you mean here :-) I can't think of a cleaner way either. Two libraries seem to be only viable option.

> One potential change is that instead of using a global argument:
>
> set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fPIC")
>
> you should consider using a per-target compile flag to the library (i.e. with
> set_properties).

You mean using set_properties on the static version? (The dynamic one is build with -fPIC anyway.) Would make any real difference? Or, alternatively, is there a problem with setting -fPIC for all sources?

[Jussi Pakkanen (jpakkane)]

Putting -fPIC to global flags is bad for all the reasons global state and global variables are bad. There are also potential clashes. For example, Qt5 executables must be build with -fPIE. What happens if you define both? I don't know, nor do I really want to spend time finding out.

FPic is needed because of one library. It is the property of said library. Therefore expressing it specifically as such is the right thing to do.

[Michi Henning (michihenning)]

> FPic is needed because of one library. It is the property of said library.
> Therefore expressing it specifically as such is the right thing to do.

Fair enough, that makes sense.

[lp:~michihenning/unity-api/static-test-lib]

62. By Michi Henning on 2013-06-18

Changed build to use -fPIC only for the source files that go into the .so and .a libraries. Improved comments and fixed some typos in comments.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:62
http://jenkins.qa.ubuntu.com/job/unity-api-ci/81/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-saucy-armhf-ci/38
        deb: http://jenkins.qa.ubuntu.com/job/unity-api-saucy-armhf-ci/38/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-saucy-i386-ci/39

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-api-ci/81/rebuild

review: Approve (continuous-integration)

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2013-06-17 08:02:36 +0000
+++ CMakeLists.txt	2013-06-18 22:51:27 +0000
@@ -117,12 +117,19 @@
 # API library
 set(UNITY_API_LIB unity-api)
 
+# Static version for testing
+set(UNITY_API_STATIC_LIB unity-api-static)
+
 # Other libraries we depend on
 set(OTHER_API_LIBS )
 
 # All the libraries we need to link a normal executable that uses the Unity API
 set(LIBS ${UNITY_API_LIB} ${OTHER_API_LIBS})
 
+# All the libraries we need to link a gtest executable. (We link the tests against a static version
+# so we can do whitebox testing on internal classes.
+set(TESTLIBS ${UNITY_API_STATIC_LIB} ${OTHER_API_LIBS})
+
 # Library install prefix
 set(LIB_INSTALL_PREFIX lib/${CMAKE_LIBRARY_ARCHITECTURE})
 

=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2013-05-17 00:28:40 +0000
+++ src/CMakeLists.txt	2013-06-18 22:51:27 +0000
@@ -1,16 +1,34 @@
 add_subdirectory(unity)
 
+# Pseudo-library of object files. We need a dynamic version of the library for normal clients,
+# and a static version for the whitebox tests, so we can write unit tests for classes in the internal namespaces
+# (because, for the .so, non-public APIs are compiled with -fvisibility=hidden).
+# Everything is compiled with -fPIC, so the same object files are suitable for either library.
+# Here, we create an object library that then is used to link the static and dynamic
+# libraries, without having to compile each source file twice with different compile flags.
+
+set(UNITY_API_LIB_OBJ ${UNITY_API_LIB}-obj)
+add_library(${UNITY_API_LIB_OBJ} OBJECT ${UNITY_API_LIB_SRC})
+
+# Use the object files to make the shared library.
 set(UNITY_API_SOVERSION 0)
-
-add_library(${UNITY_API_LIB} SHARED ${UNITY_API_LIB_SRC})
+add_library(${UNITY_API_LIB} SHARED $<TARGET_OBJECTS:${UNITY_API_LIB_OBJ}>)
 set_target_properties(${UNITY_API_LIB} PROPERTIES
     VERSION "${UNITY_API_MAJOR}.${UNITY_API_MINOR}"
     SOVERSION ${UNITY_API_SOVERSION}
 )
 
+# Use the object files to make the static library. We add -fPIC to avoid compiling a second time.
+set_source_files_properties(${UNITY_API_LIB_SRC} PROPERTIES COMPILE_FLAGS "-fPIC")
+add_library(${UNITY_API_STATIC_LIB} STATIC $<TARGET_OBJECTS:${UNITY_API_LIB_OBJ}>)
+set_target_properties(${UNITY_API_STATIC_LIB} PROPERTIES OUTPUT_NAME ${UNITY_API_LIB})
+
+# Only the dynamic library gets installed.
 install(TARGETS ${UNITY_API_LIB} LIBRARY DESTINATION ${LIB_INSTALL_PREFIX})
 
+# Set up package config.
 configure_file(lib${UNITY_API_LIB}.pc.in lib${UNITY_API_LIB}.pc @ONLY)
 install(FILES ${CMAKE_CURRENT_BINARY_DIR}/lib${UNITY_API_LIB}.pc DESTINATION ${LIB_INSTALL_PREFIX}/pkgconfig)
 
+# Parent needs to know what all the source files are, for generating doc and the like.
 set(UNITY_API_LIB_SRC ${UNITY_API_LIB_SRC} ${UNITY_SRC} PARENT_SCOPE)

=== modified file 'test/gtest/CMakeLists.txt'
--- test/gtest/CMakeLists.txt	2013-05-13 11:39:49 +0000
+++ test/gtest/CMakeLists.txt	2013-06-18 22:51:27 +0000
@@ -1,7 +1,7 @@
 find_package(Threads REQUIRED)
 set(TESTLIBDIR ${CMAKE_BINARY_DIR}/test/gtest/libgtest/build)
 set(LIBGTEST gtest)
-set(TESTLIBS ${LIBGTEST} ${CMAKE_THREAD_LIBS_INIT})
+set(TESTLIBS ${TESTLIBS} ${LIBGTEST} ${CMAKE_THREAD_LIBS_INIT})
 
 add_subdirectory(libgtest)
 add_subdirectory(unity)
@@ -12,7 +12,7 @@
 foreach(src ${TEST_SRC})
     get_filename_component(name ${src} NAME_WE)
     add_executable(${name} ${src})
-    target_link_libraries(${name} ${LIBS} ${TESTLIBS})
+    target_link_libraries(${name} ${TESTLIBS})
     string(REPLACE "_test" "" test_name ${name})
     add_test(${test_name} ${name})
 endforeach(src)

=== modified file 'test/gtest/unity/CMakeLists.txt'
--- test/gtest/unity/CMakeLists.txt	2013-05-13 11:39:49 +0000
+++ test/gtest/unity/CMakeLists.txt	2013-06-18 22:51:27 +0000
@@ -3,6 +3,6 @@
 add_subdirectory(util)
 
 add_executable(Exceptions_test Exceptions_test.cpp)
-target_link_libraries(Exceptions_test ${LIBS} ${TESTLIBS})
+target_link_libraries(Exceptions_test ${TESTLIBS})
 
 add_test(Exceptions Exceptions_test)

=== modified file 'test/gtest/unity/util/Daemon/CMakeLists.txt'
--- test/gtest/unity/util/Daemon/CMakeLists.txt	2013-05-13 11:39:49 +0000
+++ test/gtest/unity/util/Daemon/CMakeLists.txt	2013-06-18 22:51:27 +0000
@@ -1,4 +1,4 @@
 add_executable(Daemon_test Daemon_test.cpp)
-target_link_libraries(Daemon_test ${LIBS} ${TESTLIBS})
+target_link_libraries(Daemon_test ${TESTLIBS})
 
 add_test(Daemon ${CMAKE_CURRENT_SOURCE_DIR}/daemon-tester.py ${CMAKE_CURRENT_BINARY_DIR}/Daemon_test)

=== modified file 'test/gtest/unity/util/DefinesPtrs/CMakeLists.txt'
--- test/gtest/unity/util/DefinesPtrs/CMakeLists.txt	2013-05-13 11:39:49 +0000
+++ test/gtest/unity/util/DefinesPtrs/CMakeLists.txt	2013-06-18 22:51:27 +0000
@@ -1,4 +1,4 @@
 add_executable(DefinesPtrs_test DefinesPtrs_test.cpp)
-target_link_libraries(DefinesPtrs_test ${LIBS} ${TESTLIBS})
+target_link_libraries(DefinesPtrs_test ${TESTLIBS})
 
 add_test(DefinesPtrs DefinesPtrs_test)

=== modified file 'test/gtest/unity/util/FileIO/CMakeLists.txt'
--- test/gtest/unity/util/FileIO/CMakeLists.txt	2013-05-13 11:39:49 +0000
+++ test/gtest/unity/util/FileIO/CMakeLists.txt	2013-06-18 22:51:27 +0000
@@ -1,4 +1,4 @@
 add_executable(FileIO_test FileIO_test.cpp)
-target_link_libraries(FileIO_test ${LIBS} ${TESTLIBS})
+target_link_libraries(FileIO_test ${TESTLIBS})
 
 add_test(FileIO FileIO_test)

=== modified file 'test/gtest/unity/util/ResourcePtr/CMakeLists.txt'
--- test/gtest/unity/util/ResourcePtr/CMakeLists.txt	2013-05-29 04:45:19 +0000
+++ test/gtest/unity/util/ResourcePtr/CMakeLists.txt	2013-06-18 22:51:27 +0000
@@ -1,4 +1,4 @@
 add_executable(ResourcePtr_test ResourcePtr_test.cpp)
-target_link_libraries(ResourcePtr_test ${LIBS} ${TESTLIBS})
+target_link_libraries(ResourcePtr_test ${TESTLIBS})
 
 add_test(ResourcePtr ResourcePtr_test)