Merge lp:~tdaitx/location-service/lp-1640320-ftbfs-fix into lp:location-service/trunk

Proposed by Tiago Stürmer Daitx on 2017-03-11
Status: Approved
Approved by: Thomas Voß on 2017-03-11
Approved revision: 281
Proposed branch: lp:~tdaitx/location-service/lp-1640320-ftbfs-fix
Merge into: lp:location-service/trunk
Diff against target: 97 lines (+8/-14)
5 files modified
debian/control (+1/-0)
debian/control.in (+1/-0)
include/location_service/com/ubuntu/location/update.h (+1/-0)
src/location_service/com/ubuntu/location/providers/gps/android_hardware_abstraction_layer.h (+1/-0)
tests/CMakeLists.txt (+4/-14)
To merge this branch: bzr merge lp:~tdaitx/location-service/lp-1640320-ftbfs-fix
Reviewer Review Type Date Requested Status
Thomas Voß (community) 2017-03-11 Approve on 2017-03-11
Review via email: mp+319636@code.launchpad.net

Description of the change

Fix location-service FTBFS on zesty, mainly tracked on LP: #1640320

There are 2 compound causes for the FTBFS:
1. googletest update that caused cmake errors
2. missing vector includes that caused undefined reference errors

== googletest update issues ==
The googletest updated error is better described on LP: #1644062. The same fix applied to cmake-extras [1] (see comment #3 [2]) allows to detect and set the right paths for gtest and gmock, but then it runs into the same problem as comment #7 [3] which again causes the build to fail because dh_install complains about the extra files installed by googletest. This issue is better described by the commit that introduced the googletest install clause [4] and the comments there indicate that bundling googletest is not recomended - it should instead be added as an external project. This is exactly the approach taken by GMockConfig.make [5] file in cmake-extras does. The proposed solution for the first part of the FTBFS fix is to rely on that to properly reference and include both gtest and gmock.

== missing vector includes ==
The missing includes for vector causes yet another FTBFS. The includes used in this merge are simple a suggestion, I'm not familiar with location-service code so it is very likely that there is a better place to put those (or even use a different include).

[1] https://code.launchpad.net/~michihenning/cmake-extras/gtest-fixes/+merge/311565
[2] https://bugs.launchpad.net/ubuntu/+source/googletest/+bug/1644062/comments/3
[3] https://bugs.launchpad.net/ubuntu/+source/googletest/+bug/1644062/comments/7
[4] https://github.com/google/googletest/commit/98d988deac06637364f6cd41c45c3db4a8a0b6bc
[5] http://bazaar.launchpad.net/~cmake-extras/cmake-extras/trunk/view/head:/src/GMock/GMockConfig.cmake

To post a comment you must log in.
Thomas Voß (thomas-voss) wrote :

LGTM. Would you mind proposing against lp:location-service/trunk, too?

review: Approve
Balint Reczey (rbalint) wrote :

If we rely on specific cmake-extras/googletest versions should not we add versioned build-dependencies?

Will this be merged?

In case it won't there is an other smaller fix which would let the boost1.62 transition [1] finish:
https://code.launchpad.net/~rbalint/location-service/lp-1640320-fix/+merge/320805

[1] http://people.canonical.com/~ubuntu-archive/transitions/html/boost1.62.html

Thomas Voß (thomas-voss) wrote :

Yup, aiming to merge today, tomorrow the latest. Will also take care of landing to zesty.

Thomas Voß (thomas-voss) wrote :

Please add cmake-extras in snapcraft.yaml, too. Test your build with 'snapcraft cleanbuild'.

Unmerged revisions

281. By Tiago Stürmer Daitx on 2017-03-11

Fix zesty FTBFS by adding the missing vector includes. (LP: #1640320)

280. By Tiago Stürmer Daitx on 2017-03-11

Fix zesty FTBFS by replacing the bundling of googletest with find_package
from cmake-extra. (LP: #1640320) (LP: #1644062)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/control'
2--- debian/control 2016-09-12 19:16:19 +0000
3+++ debian/control 2017-03-11 05:36:04 +0000
4@@ -9,6 +9,7 @@
5 Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
6 XSBC-Original-Maintainer: Thomas Voß <thomas.voss@canonical.com>
7 Build-Depends: cmake,
8+ cmake-extras,
9 curl,
10 libdbus-cpp-dev (>= 4.1.0),
11 debhelper (>= 9),
12
13=== modified file 'debian/control.in'
14--- debian/control.in 2016-08-30 11:18:34 +0000
15+++ debian/control.in 2017-03-11 05:36:04 +0000
16@@ -4,6 +4,7 @@
17 Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
18 XSBC-Original-Maintainer: Thomas Voß <thomas.voss@canonical.com>
19 Build-Depends: cmake,
20+ cmake-extras,
21 curl,
22 libdbus-cpp-dev (>= 4.1.0),
23 debhelper (>= 9),
24
25=== modified file 'include/location_service/com/ubuntu/location/update.h'
26--- include/location_service/com/ubuntu/location/update.h 2013-12-10 09:42:54 +0000
27+++ include/location_service/com/ubuntu/location/update.h 2017-03-11 05:36:04 +0000
28@@ -21,6 +21,7 @@
29 #include <com/ubuntu/location/clock.h>
30
31 #include <ostream>
32+#include <vector>
33
34 namespace com
35 {
36
37=== modified file 'src/location_service/com/ubuntu/location/providers/gps/android_hardware_abstraction_layer.h'
38--- src/location_service/com/ubuntu/location/providers/gps/android_hardware_abstraction_layer.h 2016-02-01 15:58:06 +0000
39+++ src/location_service/com/ubuntu/location/providers/gps/android_hardware_abstraction_layer.h 2017-03-11 05:36:04 +0000
40@@ -22,6 +22,7 @@
41 #include <com/ubuntu/location/providers/gps/sntp_client.h>
42
43 #include <ubuntu/hardware/gps.h>
44+#include <vector>
45
46 namespace com { namespace ubuntu { namespace location { namespace providers { namespace gps
47 {
48
49=== modified file 'tests/CMakeLists.txt'
50--- tests/CMakeLists.txt 2016-06-11 19:58:43 +0000
51+++ tests/CMakeLists.txt 2017-03-11 05:36:04 +0000
52@@ -21,11 +21,7 @@
53 PROPERTIES COMPILE_FLAGS "-std=c99")
54
55 # Build with system gmock and embedded gtest
56-set (GMOCK_INCLUDE_DIR "/usr/include/gmock/include" CACHE PATH "gmock source include directory")
57-set (GMOCK_SOURCE_DIR "/usr/src/gmock" CACHE PATH "gmock source directory")
58-set (GTEST_INCLUDE_DIR "${GMOCK_SOURCE_DIR}/gtest/include" CACHE PATH "gtest source include directory")
59-
60-add_subdirectory(${GMOCK_SOURCE_DIR} "${CMAKE_CURRENT_BINARY_DIR}/gmock")
61+find_package(GMock)
62
63 macro(LOCATION_SERVICE_ADD_TEST test_name src)
64 add_executable(
65@@ -49,10 +45,8 @@
66
67 ${ARGN}
68
69- gmock
70-
71- gtest
72- gtest_main)
73+ ${GMOCK_LIBRARIES}
74+ ${GTEST_BOTH_LIBRARIES})
75
76 add_test(${test_name} ${CMAKE_CURRENT_BINARY_DIR}/${test_name} --gtest_filter=*-*requires*)
77
78@@ -65,9 +59,6 @@
79
80 include_directories (
81 ${CMAKE_SOURCE_DIR}/src/location_service
82-
83- ${GMOCK_INCLUDE_DIR}
84- ${GTEST_INCLUDE_DIR}
85 )
86
87 LOCATION_SERVICE_ADD_TEST(acceptance_tests acceptance_tests.cpp)
88@@ -113,8 +104,7 @@
89 ${Boost_LIBRARIES}
90 ${PROCESS_CPP_LIBRARIES}
91
92- gtest
93- gtest_main
94+ ${GTEST_BOTH_LIBRARIES})
95 )
96 if (LOCATION_SERVICE_ENABLE_DBUS_TEST_RUNNER)
97 add_test(geoclue_provider_test ${DBUS_TEST_RUNNER_EXECUTABLE} --task=${CMAKE_CURRENT_BINARY_DIR}/geoclue_provider_test)

Subscribers

People subscribed via source and target branches