Mir

Merge lp:~vanvugt/mir/test-pkgconfig into lp:mir

Proposed by Daniel van Vugt
Status: Rejected
Rejected by: Daniel van Vugt
Proposed branch: lp:~vanvugt/mir/test-pkgconfig
Merge into: lp:mir
Diff against target: 172 lines (+140/-0)
5 files modified
tests/CMakeLists.txt (+1/-0)
tests/pkgconfig/CMakeLists.txt (+11/-0)
tests/pkgconfig/standaloneclient/CMakeLists.txt (+45/-0)
tests/pkgconfig/standaloneserver/CMakeLists.txt (+45/-0)
tests/pkgconfig/standaloneserver/server.cpp (+38/-0)
To merge this branch: bzr merge lp:~vanvugt/mir/test-pkgconfig
Reviewer Review Type Date Requested Status
Daniel van Vugt Needs Fixing
PS Jenkins bot (community) continuous-integration Approve
Alan Griffiths Needs Fixing
Review via email: mp+229756@code.launchpad.net

Commit message

Add test coverage to exercise mirclient.pc.in and mirserver.pc.in and to
verify that they will work in external projects for building clients and
servers. (LP: #1352757)

Description of the change

This way we don't have to find out after-the-fact that qtmir/usc/gtk-3 aren't buildable any more.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

50 +set(LIBDIR ${Mir_BINARY_DIR}/lib)
51 +set(INCLUDEDIR ${Mir_SOURCE_DIR}/include/client)
52 +set(COMMON_INCLUDEDIR ${Mir_SOURCE_DIR}/include/shared)
53 +configure_file(${Mir_SOURCE_DIR}/src/client/mirclient.pc.in
54 + ${StandaloneClient_BINARY_DIR}/mirclient.pc)
55 +set(ENV{PKG_CONFIG_PATH} ${StandaloneClient_BINARY_DIR})

This is the code that is tested. Not the code in ${Mir_SOURCE_DIR}/src/client/CMakeLists.txt that generates the real mirclient.pc. Similarly the mirserver.pc code.

I'm not sure what the right way to test is, but cut, paste & edit of the code setting these variables before configure_file() isn't enough.

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

Alan,

Yes it's an imperfect build-time test for the pkg-config source files *.pc.in

I think you're asking for a system test of the final released packages files. That's ideal, but also not necessary. I'm still testing real *.pc files here made from the same pc input text. The fact that the paths in the tested pc files are temporary ones isn't a bug but a feature. It allows us to test the correctness of our pkg-config within the build tree, before final installation. Although you could use the real pc files with a fake root into the build tree, the result is the same (albeit more awkward) than what's proposed here.

More importantly at the end of the exercise I came to realize that CMake's insertion of temporary hard-coded search paths (rpath) actually makes any build-time testing of *.pc insufficient either way. Because the build-time search paths are literally hacked out just before packaging (thanks CMake). So the results of final package testing could easily be different.

This proposal is a basic indication of major problems in *.pc.in but it won't catch everything. And for the reason just explained, even using the real *.pc output files in a fake root would be equally insufficient, unfortunately. You either have to test the final released packages (which we do later of course), or accept that what you're testing before packaging is not the same set of binaries with the same search paths as what is about to be released.

lp:~vanvugt/mir/test-pkgconfig updated
1833. By Daniel van Vugt

Merge latest development-branch

1834. By Daniel van Vugt

Merge latest development-branch

1835. By Daniel van Vugt

Add missing mirplatform.pc, which is now a requirement again.

Revision history for this message
Chris Halse Rogers (raof) wrote :

Hm. You actually could do a proper test - invoke “make install DESTDIR=tmp”, then set pkg-config to “pkg-config --define-variable=prefix=/where/we/are/tmp/” and set PKG_CONFIG_PATH appropriately.

CMake would then nicely strip the rpath from the libraries, and you'd be testing the .pc files as-installed.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

OK, we're seeing linkage regressions that these tests are failing to notice: bug 1355021

That's probably because of the rpath madness I mentioned. I think this proposal is presently too impotent to be useful...

review: Needs Fixing

Unmerged revisions

1835. By Daniel van Vugt

Add missing mirplatform.pc, which is now a requirement again.

1834. By Daniel van Vugt

Merge latest development-branch

1833. By Daniel van Vugt

Merge latest development-branch

1832. By Daniel van Vugt

Actual working detection of cross-compiling.

1831. By Daniel van Vugt

Don't try testing pkg-config projects when cross compiling. Our cross-compile
environment is a little bit too wacky right now (many include and lib paths
don't exist).

1830. By Daniel van Vugt

Add tests for mirclient.pc

1829. By Daniel van Vugt

Remove old experimental examples/* changes

1828. By Daniel van Vugt

Much improved, and stricter testing of mirserver.pc

1827. By Daniel van Vugt

First working prototype test project that exercises mirserver.pc.in

1826. By Daniel van Vugt

First attempt at getting examples/ to use *.pc(.in)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/CMakeLists.txt'
2--- tests/CMakeLists.txt 2014-07-02 06:29:24 +0000
3+++ tests/CMakeLists.txt 2014-08-08 06:13:52 +0000
4@@ -50,5 +50,6 @@
5 add_subdirectory(mir_test_doubles/)
6 add_subdirectory(client-language/)
7 add_subdirectory(mir-stress/)
8+add_subdirectory(pkgconfig)
9
10 mir_add_memcheck_test()
11
12=== added directory 'tests/pkgconfig'
13=== added file 'tests/pkgconfig/CMakeLists.txt'
14--- tests/pkgconfig/CMakeLists.txt 1970-01-01 00:00:00 +0000
15+++ tests/pkgconfig/CMakeLists.txt 2014-08-08 06:13:52 +0000
16@@ -0,0 +1,11 @@
17+
18+# FIXME: Can't run pkg-config build testing when cross-compiling. It tries
19+# to use a bunch of paths under MIR_NDK_PATH that actually don't exist.
20+# That probably needs fixing in our cross-compile setup scripts, but
21+# so long as these test projects get attempted on native builds,
22+# that is sufficient test coverage.
23+
24+if (NOT CMAKE_FIND_ROOT_PATH_MODE_PROGRAM MATCHES "NEVER")
25+ add_subdirectory(standaloneserver)
26+ add_subdirectory(standaloneclient)
27+endif()
28
29=== added directory 'tests/pkgconfig/standaloneclient'
30=== added file 'tests/pkgconfig/standaloneclient/CMakeLists.txt'
31--- tests/pkgconfig/standaloneclient/CMakeLists.txt 1970-01-01 00:00:00 +0000
32+++ tests/pkgconfig/standaloneclient/CMakeLists.txt 2014-08-08 06:13:52 +0000
33@@ -0,0 +1,45 @@
34+#
35+# This project tests the correctness of mirclient.pc.in, ensuring it contains
36+# everything an external project needs to successfully compile and link
37+# a mir client.
38+#
39+project(StandaloneClient)
40+
41+# Forget everything you know about the parent "Mir" project. Pretend you're
42+# a distant third-party project. This ensures the directories we do get are
43+# coming from mirclient.pc ...
44+set_directory_properties(PROPERTIES
45+ INCLUDE_DIRECTORIES ""
46+ LINK_DIRECTORIES ""
47+)
48+
49+# Generate a temporary mirclient.pc file which points into the build tree ...
50+set(LIBDIR ${Mir_BINARY_DIR}/lib)
51+set(INCLUDEDIR ${Mir_SOURCE_DIR}/include/client)
52+set(COMMON_INCLUDEDIR ${Mir_SOURCE_DIR}/include/shared)
53+configure_file(${Mir_SOURCE_DIR}/src/client/mirclient.pc.in
54+ ${StandaloneClient_BINARY_DIR}/mirclient.pc)
55+set(ENV{PKG_CONFIG_PATH} ${StandaloneClient_BINARY_DIR})
56+
57+# Now use mirclient.pc like real projects do, verifying it provides all
58+# the necessary information to build...
59+find_package(PkgConfig)
60+pkg_check_modules(LOCALMIRCLIENT REQUIRED mirclient)
61+
62+include_directories(${LOCALMIRCLIENT_INCLUDE_DIRS})
63+link_directories(${LOCALMIRCLIENT_LIBRARY_DIRS})
64+add_compile_options(${LOCALMIRCLIENT_CFLAGS})
65+
66+set(CLIENT_SRC "${Mir_SOURCE_DIR}/examples/flicker.c")
67+add_compile_options("-std=c99")
68+
69+# Test CMake-style explicit linkage: "foo/libbar.so"
70+add_executable(test_standaloneclient_directlink ${CLIENT_SRC})
71+add_dependencies(test_standaloneclient_directlink mirclient)
72+target_link_libraries(test_standaloneclient_directlink ${LOCALMIRCLIENT_LIBRARIES})
73+
74+# Test traditional linkage: "-Lfoo -lbar"
75+add_executable(test_standaloneclient_searchlink ${CLIENT_SRC})
76+add_dependencies(test_standaloneclient_searchlink mirclient)
77+target_link_libraries(test_standaloneclient_searchlink ${LOCALMIRCLIENT_LDFLAGS})
78+
79
80=== added directory 'tests/pkgconfig/standaloneserver'
81=== added file 'tests/pkgconfig/standaloneserver/CMakeLists.txt'
82--- tests/pkgconfig/standaloneserver/CMakeLists.txt 1970-01-01 00:00:00 +0000
83+++ tests/pkgconfig/standaloneserver/CMakeLists.txt 2014-08-08 06:13:52 +0000
84@@ -0,0 +1,45 @@
85+#
86+# This project tests the correctness of mirserver.pc.in, ensuring it contains
87+# everything an external project needs to successfully compile and link
88+# a mir server.
89+#
90+project(StandaloneServer)
91+
92+# Forget everything you know about the parent "Mir" project. Pretend you're
93+# a distant third-party project. This ensures the directories we do get are
94+# coming from mirserver.pc ...
95+set_directory_properties(PROPERTIES
96+ INCLUDE_DIRECTORIES ""
97+ LINK_DIRECTORIES ""
98+)
99+
100+# Generate a temporary mirserver.pc file which points into the build tree ...
101+set(LIBDIR ${Mir_BINARY_DIR}/lib)
102+set(INCLUDEDIR ${Mir_SOURCE_DIR}/include/server)
103+set(PLATFORM_INCLUDEDIR ${Mir_SOURCE_DIR}/include/platform)
104+set(COMMON_INCLUDEDIR ${Mir_SOURCE_DIR}/include/shared)
105+configure_file(${Mir_SOURCE_DIR}/src/platform/mirplatform.pc.in
106+ ${StandaloneServer_BINARY_DIR}/mirplatform.pc)
107+configure_file(${Mir_SOURCE_DIR}/src/server/mirserver.pc.in
108+ ${StandaloneServer_BINARY_DIR}/mirserver.pc)
109+set(ENV{PKG_CONFIG_PATH} ${StandaloneServer_BINARY_DIR})
110+
111+# Now use mirserver.pc like real projects do, verifying it provides all
112+# the necessary information to build...
113+find_package(PkgConfig)
114+pkg_check_modules(LOCALMIRSERVER REQUIRED mirserver)
115+
116+include_directories(${LOCALMIRSERVER_INCLUDE_DIRS})
117+link_directories(${LOCALMIRSERVER_LIBRARY_DIRS})
118+add_compile_options(${LOCALMIRSERVER_CFLAGS})
119+
120+# Test CMake-style explicit linkage: "foo/libbar.so"
121+add_executable(test_standaloneserver_directlink server.cpp)
122+add_dependencies(test_standaloneserver_directlink mirserver)
123+target_link_libraries(test_standaloneserver_directlink ${LOCALMIRSERVER_LIBRARIES})
124+
125+# Test traditional linkage: "-Lfoo -lbar"
126+add_executable(test_standaloneserver_searchlink server.cpp)
127+add_dependencies(test_standaloneserver_searchlink mirserver)
128+target_link_libraries(test_standaloneserver_searchlink ${LOCALMIRSERVER_LDFLAGS})
129+
130
131=== added file 'tests/pkgconfig/standaloneserver/server.cpp'
132--- tests/pkgconfig/standaloneserver/server.cpp 1970-01-01 00:00:00 +0000
133+++ tests/pkgconfig/standaloneserver/server.cpp 2014-08-08 06:13:52 +0000
134@@ -0,0 +1,38 @@
135+/*
136+ * Copyright © 2014 Canonical Ltd.
137+ *
138+ * This program is free software: you can redistribute it and/or modify
139+ * it under the terms of the GNU 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+ *
147+ * You should have received a copy of the GNU General Public License
148+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
149+ *
150+ * Authored by: Daniel van Vugt <daniel.van.vugt@canonical.com>
151+ */
152+
153+#include "mir/default_server_configuration.h"
154+#include "mir/run_mir.h"
155+#include "mir/geometry/rectangle.h"
156+
157+using namespace mir;
158+using namespace mir::geometry;
159+
160+int main(int argc, char const* argv[])
161+{
162+ // Exercise some symbols from libmircommon. Make sure users of
163+ // mirserver.pc get these automatically in their link lines...
164+ Rectangle a;
165+ (void)a.bottom_right();
166+ Rectangle b;
167+ (void)a.contains(b);
168+
169+ DefaultServerConfiguration config(argc, argv);
170+ run_mir(config, [](mir::DisplayServer&){} );
171+ return 0;
172+}

Subscribers

People subscribed via source and target branches