Mir

Merge lp:~vanvugt/mir/fix-1283951 into lp:mir

  1. fix-1283951
  2. Merge into development-branch
Proposed by Daniel van Vugt
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
Merged at revision: 1436
Proposed branch: lp:~vanvugt/mir/fix-1283951
Merge into: lp:mir
Diff against target: 275 lines (+31/-30)
14 files modified
examples/CMakeLists.txt (+0/-1)
examples/render_overlays.cpp (+2/-2)
include/shared/testdraw/draw_pattern_checkered-inl.h (+3/-3)
include/shared/testdraw/graphics_region_factory.h (+3/-3)
include/shared/testdraw/patterns.h (+3/-3)
src/shared/CMakeLists.txt (+1/-0)
src/shared/testdraw/CMakeLists.txt (+2/-0)
src/shared/testdraw/android_graphics_region_factory.cpp (+4/-4)
src/shared/testdraw/mesa_graphics_region_factory.cpp (+4/-4)
src/shared/testdraw/patterns.cpp (+4/-4)
tests/CMakeLists.txt (+0/-1)
tests/integration-tests/client/test_client_render.cpp (+2/-2)
tests/integration-tests/graphics/android/test_buffer_integration.cpp (+2/-2)
tests/unit-tests/draw/test_draw_patterns.cpp (+1/-1)
To merge this branch: bzr merge lp:~vanvugt/mir/fix-1283951
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alan Griffiths Approve
Kevin DuBois (community) Approve
Alexandros Frantzis Pending
Review via email: mp+208069@code.launchpad.net

This proposal supersedes a proposal from 2014-02-24.

Commit message

Fix FTBFS in examples/ when MIR_ENABLE_TESTS=OFF (LP: #1283951)

To post a comment you must log in.
Revision history for this message
Alexandros Frantzis (afrantzis) wrote : Posted in a previous version of this proposal

Building examples shouldn't depend on tests being enabled. Making this dependency explicit, although an improvement over the current state, is only a band-aid fix. Since a fix is not urgently, I think we should do it the right way and move mirtestdraw functionality to somewhere where both examples and tests can use it independently.

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:1420
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/926/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1020
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1018
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/612/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/656
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/656/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/663
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/663/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/613
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/613/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/587/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4217

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/926/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

> Building examples shouldn't depend on tests being enabled. Making this
> dependency explicit, although an improvement over the current state, is only a
> band-aid fix. Since a fix is not urgently, I think we should do it the right
> way and move mirtestdraw functionality to somewhere where both examples and
> tests can use it independently.

Agreed. It makes more sense for tests to use example code than for examples to use test code.

IIRC the reason we have this mess is that mirtestdraw hooks into some private APIs (that either shouldn't be used by example code, shouldn't be private, or preferably should be reworked to expose useful functionality).

However, this "fix" avoids a FTBS without making the mess worse.

review: Approve
Revision history for this message
Kevin DuBois (kdub) wrote : Posted in a previous version of this proposal

> IIRC the reason we have this mess is that mirtestdraw hooks into some private
> APIs (that either shouldn't be used by example code, shouldn't be private, or
> preferably should be reworked to expose useful functionality).

Its more that its platform-specific code (android software buffer mapping) and that this is used in the tests and the example code. Fix looks good to me.

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Rewrote per Alexandros' request. libmirtestdraw is now built separately to tests so examples can always use it.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1422
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/932/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1033
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1031
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/625
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/662/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/669
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/669/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/626
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/626/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/600
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4263

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/932/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1423
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/940/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1041
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1039
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/633
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/672
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/672/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/677
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/677/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/634
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/634/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/608
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4302

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/940/rebuild

review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

still okay

review: Approve
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

There's no obvious place for this code. I guess this will do for now.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'examples/CMakeLists.txt'
2--- examples/CMakeLists.txt 2014-01-29 22:02:57 +0000
3+++ examples/CMakeLists.txt 2014-02-26 03:23:52 +0000
4@@ -93,7 +93,6 @@
5 ${PROJECT_SOURCE_DIR}/include/server
6 ${PROJECT_SOURCE_DIR}/include/client
7 ${PROJECT_SOURCE_DIR}/include/platform
8- ${PROJECT_SOURCE_DIR}/include/test/mir_test/draw
9 ${GLESv2_INCLUDE_DIRS}
10 )
11
12
13=== modified file 'examples/render_overlays.cpp'
14--- examples/render_overlays.cpp 2014-02-14 09:27:02 +0000
15+++ examples/render_overlays.cpp 2014-02-26 03:23:52 +0000
16@@ -25,8 +25,8 @@
17 #include "mir/graphics/buffer_properties.h"
18 #include "mir/report_exception.h"
19
20-#include "graphics_region_factory.h"
21-#include "patterns.h"
22+#include "testdraw/graphics_region_factory.h"
23+#include "testdraw/patterns.h"
24
25 #include <csignal>
26 #include <iostream>
27
28=== renamed directory 'include/test/mir_test/draw' => 'include/shared/testdraw'
29=== modified file 'include/shared/testdraw/draw_pattern_checkered-inl.h'
30--- include/test/mir_test/draw/draw_pattern_checkered-inl.h 2014-01-13 06:12:33 +0000
31+++ include/shared/testdraw/draw_pattern_checkered-inl.h 2014-02-26 03:23:52 +0000
32@@ -2,15 +2,15 @@
33 * Copyright © 2012 Canonical Ltd.
34 *
35 * This program is free software: you can redistribute it and/or modify it
36- * under the terms of the GNU General Public License version 3,
37+ * under the terms of the GNU Lesser General Public License version 3,
38 * as published by the Free Software Foundation.
39 *
40 * This program is distributed in the hope that it will be useful,
41 * but WITHOUT ANY WARRANTY; without even the implied warranty of
42 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
43- * GNU General Public License for more details.
44+ * GNU Lesser General Public License for more details.
45 *
46- * You should have received a copy of the GNU General Public License
47+ * You should have received a copy of the GNU Lesser General Public License
48 * along with this program. If not, see <http://www.gnu.org/licenses/>.
49 *
50 * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
51
52=== modified file 'include/shared/testdraw/graphics_region_factory.h'
53--- include/test/mir_test/draw/graphics_region_factory.h 2014-01-28 21:11:22 +0000
54+++ include/shared/testdraw/graphics_region_factory.h 2014-02-26 03:23:52 +0000
55@@ -2,15 +2,15 @@
56 * Copyright © 2012 Canonical Ltd.
57 *
58 * This program is free software: you can redistribute it and/or modify it
59- * under the terms of the GNU General Public License version 3,
60+ * under the terms of the GNU Lesser General Public License version 3,
61 * as published by the Free Software Foundation.
62 *
63 * This program is distributed in the hope that it will be useful,
64 * but WITHOUT ANY WARRANTY; without even the implied warranty of
65 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
66- * GNU General Public License for more details.
67+ * GNU Lesser General Public License for more details.
68 *
69- * You should have received a copy of the GNU General Public License
70+ * You should have received a copy of the GNU Lesser General Public License
71 * along with this program. If not, see <http://www.gnu.org/licenses/>.
72 *
73 * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
74
75=== modified file 'include/shared/testdraw/patterns.h'
76--- include/test/mir_test/draw/patterns.h 2014-01-28 21:00:55 +0000
77+++ include/shared/testdraw/patterns.h 2014-02-26 03:23:52 +0000
78@@ -2,15 +2,15 @@
79 * Copyright © 2012 Canonical Ltd.
80 *
81 * This program is free software: you can redistribute it and/or modify it
82- * under the terms of the GNU General Public License version 3,
83+ * under the terms of the GNU Lesser General Public License version 3,
84 * as published by the Free Software Foundation.
85 *
86 * This program is distributed in the hope that it will be useful,
87 * but WITHOUT ANY WARRANTY; without even the implied warranty of
88 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
89- * GNU General Public License for more details.
90+ * GNU Lesser General Public License for more details.
91 *
92- * You should have received a copy of the GNU General Public License
93+ * You should have received a copy of the GNU Lesser General Public License
94 * along with this program. If not, see <http://www.gnu.org/licenses/>.
95 *
96 * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
97
98=== modified file 'src/shared/CMakeLists.txt'
99--- src/shared/CMakeLists.txt 2014-02-19 14:01:37 +0000
100+++ src/shared/CMakeLists.txt 2014-02-26 03:23:52 +0000
101@@ -12,6 +12,7 @@
102 add_subdirectory(report/lttng)
103 add_subdirectory(env)
104 add_subdirectory(sharedlibrary)
105+add_subdirectory(testdraw)
106
107 set(
108 MIR_COMMON_PLATFORM_LIBRARIES
109
110=== renamed directory 'tests/draw' => 'src/shared/testdraw'
111=== modified file 'src/shared/testdraw/CMakeLists.txt'
112--- tests/draw/CMakeLists.txt 2014-02-14 09:27:02 +0000
113+++ src/shared/testdraw/CMakeLists.txt 2014-02-26 03:23:52 +0000
114@@ -2,6 +2,7 @@
115 ${Boost_INCLUDE_DIRS}
116 ${GLESv2_INCLUDE_DIRS}
117 ${CMAKE_SOURCE_DIR}
118+ ${PROJECT_SOURCE_DIR}/include/client
119 )
120
121 set(
122@@ -12,6 +13,7 @@
123
124 if (MIR_TEST_PLATFORM STREQUAL "android")
125 include_directories(SYSTEM ${LIBHARDWARE_INCLUDE_DIRS})
126+ add_definitions(-DANDROID)
127 set(DRAW_SRCS
128 android_graphics_region_factory.cpp
129 ${DRAW_SRCS})
130
131=== modified file 'src/shared/testdraw/android_graphics_region_factory.cpp'
132--- tests/draw/android_graphics_region_factory.cpp 2014-02-10 17:36:52 +0000
133+++ src/shared/testdraw/android_graphics_region_factory.cpp 2014-02-26 03:23:52 +0000
134@@ -2,21 +2,21 @@
135 * Copyright © 2012, 2014 Canonical Ltd.
136 *
137 * This program is free software: you can redistribute it and/or modify
138- * it under the terms of the GNU General Public License version 3 as
139+ * it under the terms of the GNU Lesser General Public License version 3 as
140 * published by the Free Software Foundation.
141 *
142 * This program is distributed in the hope that it will be useful,
143 * but WITHOUT ANY WARRANTY; without even the implied warranty of
144 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
145- * GNU General Public License for more details.
146+ * GNU Lesser General Public License for more details.
147 *
148- * You should have received a copy of the GNU General Public License
149+ * You should have received a copy of the GNU Lesser General Public License
150 * along with this program. If not, see <http://www.gnu.org/licenses/>.
151 *
152 * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
153 */
154
155-#include "mir_test/draw/graphics_region_factory.h"
156+#include "testdraw/graphics_region_factory.h"
157 #include "mir_toolkit/mir_client_library.h"
158 #include "mir/graphics/android/native_buffer.h"
159 #include "mir_toolkit/common.h"
160
161=== modified file 'src/shared/testdraw/mesa_graphics_region_factory.cpp'
162--- tests/draw/mesa_graphics_region_factory.cpp 2014-01-29 22:02:57 +0000
163+++ src/shared/testdraw/mesa_graphics_region_factory.cpp 2014-02-26 03:23:52 +0000
164@@ -2,21 +2,21 @@
165 * Copyright © 2014 Canonical Ltd.
166 *
167 * This program is free software: you can redistribute it and/or modify
168- * it under the terms of the GNU General Public License version 3 as
169+ * it under the terms of the GNU Lesser General Public License version 3 as
170 * published by the Free Software Foundation.
171 *
172 * This program is distributed in the hope that it will be useful,
173 * but WITHOUT ANY WARRANTY; without even the implied warranty of
174 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
175- * GNU General Public License for more details.
176+ * GNU Lesser General Public License for more details.
177 *
178- * You should have received a copy of the GNU General Public License
179+ * You should have received a copy of the GNU Lesser General Public License
180 * along with this program. If not, see <http://www.gnu.org/licenses/>.
181 *
182 * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
183 */
184
185-#include "mir_test/draw/graphics_region_factory.h"
186+#include "testdraw/graphics_region_factory.h"
187 #include <boost/throw_exception.hpp>
188 #include <stdexcept>
189
190
191=== modified file 'src/shared/testdraw/patterns.cpp'
192--- tests/draw/patterns.cpp 2014-01-13 06:12:33 +0000
193+++ src/shared/testdraw/patterns.cpp 2014-02-26 03:23:52 +0000
194@@ -2,22 +2,22 @@
195 * Copyright © 2012 Canonical Ltd.
196 *
197 * This program is free software: you can redistribute it and/or modify
198- * it under the terms of the GNU General Public License version 3 as
199+ * it under the terms of the GNU Lesser General Public License version 3 as
200 * published by the Free Software Foundation.
201 *
202 * This program is distributed in the hope that it will be useful,
203 * but WITHOUT ANY WARRANTY; without even the implied warranty of
204 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
205- * GNU General Public License for more details.
206+ * GNU Lesser General Public License for more details.
207 *
208- * You should have received a copy of the GNU General Public License
209+ * You should have received a copy of the GNU Lesser General Public License
210 * along with this program. If not, see <http://www.gnu.org/licenses/>.
211 *
212 * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
213 */
214
215 #include "mir_toolkit/common.h"
216-#include "mir_test/draw/patterns.h"
217+#include "testdraw/patterns.h"
218
219 namespace mtd=mir::test::draw;
220
221
222=== modified file 'tests/CMakeLists.txt'
223--- tests/CMakeLists.txt 2014-02-14 09:27:02 +0000
224+++ tests/CMakeLists.txt 2014-02-26 03:23:52 +0000
225@@ -50,7 +50,6 @@
226 add_subdirectory(mir_test/)
227 add_subdirectory(mir_test_framework/)
228 add_subdirectory(mir_test_doubles/)
229-add_subdirectory(draw/)
230 add_subdirectory(client-language/)
231 add_subdirectory(mir-stress/)
232
233
234=== modified file 'tests/integration-tests/client/test_client_render.cpp'
235--- tests/integration-tests/client/test_client_render.cpp 2014-02-10 23:38:46 +0000
236+++ tests/integration-tests/client/test_client_render.cpp 2014-02-26 03:23:52 +0000
237@@ -25,8 +25,8 @@
238 #include "src/platform/graphics/android/android_graphic_buffer_allocator.h"
239
240 #include "mir_test_framework/cross_process_sync.h"
241-#include "mir_test/draw/graphics_region_factory.h"
242-#include "mir_test/draw/patterns.h"
243+#include "testdraw/graphics_region_factory.h"
244+#include "testdraw/patterns.h"
245 #include "mir_test/stub_server_tool.h"
246 #include "mir_test/test_protobuf_server.h"
247
248
249=== modified file 'tests/integration-tests/graphics/android/test_buffer_integration.cpp'
250--- tests/integration-tests/graphics/android/test_buffer_integration.cpp 2014-02-19 14:01:37 +0000
251+++ tests/integration-tests/graphics/android/test_buffer_integration.cpp 2014-02-26 03:23:52 +0000
252@@ -23,8 +23,8 @@
253 #include "mir/graphics/android/native_buffer.h"
254 #include "mir/graphics/buffer_properties.h"
255
256-#include "mir_test/draw/graphics_region_factory.h"
257-#include "mir_test/draw/patterns.h"
258+#include "testdraw/graphics_region_factory.h"
259+#include "testdraw/patterns.h"
260
261 #include <gtest/gtest.h>
262
263
264=== modified file 'tests/unit-tests/draw/test_draw_patterns.cpp'
265--- tests/unit-tests/draw/test_draw_patterns.cpp 2014-01-13 06:12:33 +0000
266+++ tests/unit-tests/draw/test_draw_patterns.cpp 2014-02-26 03:23:52 +0000
267@@ -17,7 +17,7 @@
268 */
269
270 #include "mir_toolkit/mir_client_library.h"
271-#include "mir_test/draw/patterns.h"
272+#include "testdraw/patterns.h"
273
274 #include <gtest/gtest.h>
275 #include <stdexcept>

Subscribers

People subscribed via source and target branches