Merge lp:~raof/mir/buildsystems-are-the-worst into lp:mir

[Chris Halse Rogers]

Don't let CMake play with rpath, it cuts itself.

So, the awesome thing about rpath is that it PRECEEDS all the other paths.
If, just for the sake of argument, your cross compiles set an rpath to
your partial chroot you'd find that you're surprisingly loading the version of libmirclient.so.8 found in there.

Rather than letting CMake do its rpath voodoo, manually set up the rpath-link flags we need for the cross-compile and set LD_LIBRARY_PATH appropriately where needed.

Fixes the multiple inclusion of protobuf singletons caused when the system
Mir stuff is linking against one version of libmircommon and we're building
a new version.

Fixes: https://bugs.launchpad.net/mir/+bug/1358698

Revel in the weirdness of buildsystems!

[Alan Griffiths (alan-griffiths)]

I'm concerned by the need to set LD_LIBRARY_PATH - do we need to do that when running the programs from the commandline? If so, that is too much of a trap for the unwary.

review: Needs Information

[Chris Halse Rogers (raof)]

Yes, if you're running from the build tree. Obviously not once they're installed.

Setting environment variables is required for privatise-all-the-things, too.

-----Original Message-----
From: "Alan Griffiths" <email address hidden>
Sent: ‎22/‎08/‎2014 19:02
To: "Chris Halse Rogers" <email address hidden>
Subject: Re: [Merge] lp:~raof/mir/buildsystems-are-the-worst into lp:mir/devel

Review: Needs Information

I'm concerned by the need to set LD_LIBRARY_PATH - do we need to do that when running the programs from the commandline? If so, that is too much of a trap for the unwary.
--
https://code.launchpad.net/~raof/mir/buildsystems-are-the-worst/+merge/231840
You are the owner of lp:~raof/mir/buildsystems-are-the-worst.

[Alan Griffiths (alan-griffiths)]

> Yes, if you're running from the build tree. Obviously not once they're
> installed.

Then, at the very least, this needs to be documented in doc/using_mir_on_[pc|android].md.

I think it is already unfortunate that building Mir and running "bin/mir_unit_tests" doesn't "just work" as it is likely one of the the first thing anyone downloading Mir would try.

review: Needs Fixing

[Chris Halse Rogers (raof)]

For what it's worth, I'm pretty sure you currently need to set LD_LIBRARY_PATH to safely run any of the mir_demo* binaries already - mesa doesn't know anything about that rpath, so it's not clear which mirclient will be loaded.

-----Original Message-----
From: "Alan Griffiths" <email address hidden>
Sent: ‎22/‎08/‎2014 19:21
To: "Chris Halse Rogers" <email address hidden>
Subject: Re: RE: [Merge] lp:~raof/mir/buildsystems-are-the-worst intolp:mir/devel

Review: Needs Fixing

> Yes, if you're running from the build tree. Obviously not once they're
> installed.

Then, at the very least, this needs to be documented in doc/using_mir_on_[pc|android].md.

I think it is already unfortunate that building Mir and running "bin/mir_unit_tests" doesn't "just work" as it is likely one of the the first thing anyone downloading Mir would try.
--
https://code.launchpad.net/~raof/mir/buildsystems-are-the-worst/+merge/231840
You are the owner of lp:~raof/mir/buildsystems-are-the-worst.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1859
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2469/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1421/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1427
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1406/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/991
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/991/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/343
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/343/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2484/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/11949

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

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

> For what it's worth, I'm pretty sure you currently need to set LD_LIBRARY_PATH
> to safely run any of the mir_demo* binaries already

I'm not sure of the mechanism but it seems to be working at present:

Running the built binary:

$ strace bin/mir_demo_client_basic 2>&1 | grep libmirclient
open("/home/alan/display_server/mir3/build/lib/tls/x86_64/libmirclient.so.8", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
open("/home/alan/display_server/mir3/build/lib/tls/libmirclient.so.8", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
open("/home/alan/display_server/mir3/build/lib/x86_64/libmirclient.so.8", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
open("/home/alan/display_server/mir3/build/lib/libmirclient.so.8", O_RDONLY|O_CLOEXEC) = 3

Running the installed binary:

$ strace mir_demo_client_basic 2>&1 | grep libmirclient
open("/usr/lib/x86_64-linux-gnu/libmirclient.so.8", O_RDONLY|O_CLOEXEC) = 3

[Chris Halse Rogers (raof)]

Yeah, the basic demos will be fine - they've got the RPATH linked in, and don't link against anything that links against the Mir libs.

And since Mesa uses dlopen() that's also OK. So my concern is moot (at least until we have a demo that links against something that links against Mir ☺).

We really do need something like this, though. When we build using cross-compile-chroot.sh all our tests are linked against the libraries in the chroot. For example, the ldd output of libmirserver.so:

(utopic-armhf)root@RedTail:/home/chris/Canonical/Mir/mir/private-library-loading/build-android-arm# ldd lib/libmirserver.so
 /usr/lib/libeatmydata/libeatmydata.so (0xf5fcf000)
 libmirclient.so.8 => /home/chris/Canonical/Mir/mir/private-library-loading/partial-armhf-chroot/usr/lib/arm-linux-gnueabihf/libmirclient.so.8 (0xf5f8e000)
 libmirplatform.so.2 => not found
 libmircommon.so.2 => not found
 libglog.so.0 => /home/chris/Canonical/Mir/mir/private-library-loading/partial-armhf-chroot/usr/lib/arm-linux-gnueabihf/libglog.so.0 (0xf5f55000)
 libgflags.so.2 => /home/chris/Canonical/Mir/mir/private-library-loading/partial-armhf-chroot/usr/lib/arm-linux-gnueabihf/libgflags.so.2 (0xf5f2e000)
 libEGL.so.1 => not found
 libGLESv2.so.2 => not found
 libdl.so.2 => /lib/arm-linux-gnueabihf/libdl.so.2 (0xf5f1a000)
 libprotobuf.so.8 => /home/chris/Canonical/Mir/mir/private-library-loading/partial-armhf-chroot/usr/lib/arm-linux-gnueabihf/libprotobuf.so.8 (0xf5e68000)
 libpthread.so.0 => /lib/arm-linux-gnueabihf/libpthread.so.0 (0xf5e45000)
 libboost_system.so.1.55.0 => /home/chris/Canonical/Mir/mir/private-library-loading/partial-armhf-chroot/usr/lib/arm-linux-gnueabihf/libboost_system.so.1.55.0 (0xf5e32000)
 libboost_program_options.so.1.55.0 => /home/chris/Canonical/Mir/mir/private-library-loading/partial-armhf-chroot/usr/lib/arm-linux-gnueabihf/libboost_program_options.so.1.55.0 (0xf5dd5000)
 libstdc++.so.6 => /home/chris/Canonical/Mir/mir/private-library-loading/partial-armhf-chroot/usr/lib/arm-linux-gnueabihf/libstdc++.so.6 (0xf5d1b000)
 libm.so.6 => /lib/arm-linux-gnueabihf/libm.so.6 (0xf5ca7000)
 libgcc_s.so.1 => /home/chris/Canonical/Mir/mir/private-library-loading/partial-armhf-chroot/lib/arm-linux-gnueabihf/libgcc_s.so.1 (0xf5c7d000)
 libc.so.6 => /lib/arm-linux-gnueabihf/libc.so.6 (0xf5b8e000)
 /lib/ld-linux-armhf.so.3 (0xf6fd7000)
 libmircommon.so.1 => /home/chris/Canonical/Mir/mir/private-library-loading/partial-armhf-chroot/usr/lib/arm-linux-gnueabihf/libmircommon.so.1 (0xf5b23000)
 libz.so.1 => /home/chris/Canonical/Mir/mir/private-library-loading/partial-armhf-chroot/lib/arm-linux-gnueabihf/libz.so.1 (0xf5b08000)
 libxkbcommon.so.0 => /home/chris/Canonical/Mir/mir/private-library-loading/partial-armhf-chroot/usr/lib/arm-linux-gnueabihf/libxkbcommon.so.0 (0xf5acc000)
 libudev.so.1 => /home/chris/Canonical/Mir/mir/private-library-loading/partial-armhf-chroot/lib/arm-linux-gnueabihf/libudev.so.1 (0xf5ab0000)
 libselinux.so.1 => /home/chris/Canonical/Mir/mir/private-library-loading/partial-armhf-chroot/lib/arm-linux-gnueabihf/libselinux.so.1 (0xf5a91000)
 librt.so.1 => /lib/arm-linux-gnueabihf/librt.so.1 (0xf5a7b000)
 libpcre.so.3 => /home/chris/Canonical/Mir/mi...

[Daniel van Vugt (vanvugt)]

I don't mind LD_LIBRARY_PATH too much. If we were using anything other than CMake it would always be required anyway. When cross compiling you already have to remember LD_LIBRARY_PATH after coping files to your device. And actually I think CMake inserting its own rpaths has caused plenty of bugs and a false sense of security. So you win some and lose some.

[Chris Halse Rogers (raof)]

Hm. Ok, so http://s-jenkins.ubuntu-ci:8080/job/mir-mediumtests-runner-mako/2503/console proves that this does, indeed, fix the problem of linking against all the things.

And http://s-jenkins.ubuntu-ci:8080/job/mir-mediumtests-runner-mako/2506/console shows that the remaining problem is that we don't upgrade libmirplatform-graphics-android. I've got a branch of the testrunner that'll fix that, but only once Mir is parallel installable with old versions.

So I'll merge in Alan's versioning revert and then this branch should pass.

[Andreas Pokorny (andreas-pokorny)]

How is that possible?

> (utopic-armhf)root@RedTail:/home/chris/Canonical/Mir/mir/private-library-
> loading/build-android-arm# ldd lib/libmirserver.so
> /usr/lib/libeatmydata/libeatmydata.so (0xf5fcf000)
> libmirclient.so.8 => /home/chris/Canonical/Mir/mir/private-library-
> loading/partial-armhf-chroot/usr/lib/arm-linux-gnueabihf/libmirclient.so.8
> (0xf5f8e000)
> libmirplatform.so.2 => not found
> libmircommon.so.2 => not found

> [...]

and:

> libmircommon.so.1 => /home/chris/Canonical/Mir/mir/private-library-
> loading/partial-armhf-chroot/usr/lib/arm-linux-gnueabihf/libmircommon.so.1
> (0xf5b23000)

The make build files generated by CMake always just refer to ..<relative_path_to>/lib/libmircommon.so.<version_built>. There is not a single -lmircommon that could cause that..

So how can it link against two different versions of libmircommon?

[Andreas Pokorny (andreas-pokorny)]

PS: If that really solves the issue of conflicting mircommons, why not manually set rpath in the none cross compiled case?

review: Needs Information

[Chris Halse Rogers (raof)]

libmirserver gets linked with ../lib/libmirclient.so, yes. But that means that libmirserver.so gets ‘libmirclient.so.8’ added to its DT_NEEDED entries.

When the dynamic loader is resolving libraries it checks RPATH first. RPATH in this case starts with /home/chris/Canonical/Mir/mir/private-library-loading/partial-armhf-chroot/usr/lib/arm-linux-gnueabihf/ which just happens to have a libmirclient.so.8 in there! Hurray!

So it loads that one, and that libmirclient.so.8 happens to have libmircommon.so.1 encoded in one of its DT_NEEDED entries, so we load that…

I have tried to find a way for CMake to not add the partial-armhf-chroot to RPATH, but I couldn't find one.

[lp:~raof/mir/buildsystems-are-the-worst]

1860. By Chris Halse Rogers on 2014-08-27

Merge trunk

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1860
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2518/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1481/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1487
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1465
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1040
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1040/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/402
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/402/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2549
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12184

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

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

I'm not entirely convinced by the implied problem diagnosis.

On desktop cmake appears to get the rpath logic right (including relinking binaries with the right rpath when installing).

To me that suggests that cmake is fully capable of getting things right and that the problem is that the context for "installing" in the partial-chroot setup is the only thing broken.

[Chris Halse Rogers (raof)]

This is only actually a problem for local developers, not CI. See bug reports.

Unmerged revisions

1860. By Chris Halse Rogers on 2014-08-27

Merge trunk

1859. By Chris Halse Rogers on 2014-08-22

Don't let CMake play with rpath, it cuts itself.

So, the awesome thing about rpath is that it PRECEEDS all the other paths.
If, just for the sake of argument, your cross compiles set an rpath-link to
your partial chroot you'd find that you're surprisingly linking against the
version of libmirclient.so.8 found in there.

Fixes the multiple inclusion of protobuf singletons caused when the system
Mir stuff is linking against one version of libmircommon and we're building
a new version.

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2014-08-21 13:18:01 +0000
+++ CMakeLists.txt	2014-08-27 02:19:21 +0000
@@ -23,6 +23,8 @@
 
 cmake_policy(SET CMP0015 NEW)
 
+set(CMAKE_SKIP_RPATH TRUE)
+
 set(EXECUTABLE_OUTPUT_PATH ${CMAKE_BINARY_DIR}/bin)
 set(LIBRARY_OUTPUT_PATH ${CMAKE_BINARY_DIR}/lib)
 

=== modified file 'cmake/LinuxCrossCompile.cmake'
--- cmake/LinuxCrossCompile.cmake	2014-07-28 13:49:54 +0000
+++ cmake/LinuxCrossCompile.cmake	2014-08-27 02:19:21 +0000
@@ -22,11 +22,9 @@
   "${MIR_NDK_PATH}/usr/lib/${MIR_ARM_EABI}"
 )
 
-set(CMAKE_INSTALL_RPATH_USE_LINK_PATH FALSE)
-set(CMAKE_BUILD_WITH_INSTALL_RPATH TRUE)
-set(CMAKE_EXECUTABLE_RUNTIME_C_FLAG "-Wl,-rpath-link,")
-set(CMAKE_EXECUTABLE_RUNTIME_CXX_FLAG "-Wl,-rpath-link,")
-set(CMAKE_INSTALL_RPATH "${MIR_NDK_PATH}/lib:${MIR_NDK_PATH}/lib/${MIR_ARM_EABI}:${MIR_NDK_PATH}/usr/lib:${MIR_NDK_PATH}/usr/lib/${MIR_ARM_EABI}")
+set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--rpath-link,${LIBRARY_OUTPUT_PATH} -Wl,--rpath-link,${MIR_NDK_PATH}/lib -Wl,--rpath-link,${MIR_NDK_PATH}/lib/${MIR_ARM_EABI} -Wl,--rpath-link,${MIR_NDK_PATH}/usr/lib -Wl,--rpath-link,${MIR_NDK_PATH}/usr/lib/${MIR_ARM_EABI}")
+set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--rpath-link,${LIBRARY_OUTPUT_PATH} -Wl,--rpath-link,${MIR_NDK_PATH}/lib -Wl,--rpath-link,${MIR_NDK_PATH}/lib/${MIR_ARM_EABI} -Wl,--rpath-link,${MIR_NDK_PATH}/usr/lib -Wl,--rpath-link,${MIR_NDK_PATH}/usr/lib/${MIR_ARM_EABI}")
+set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--rpath-link,${LIBRARY_OUTPUT_PATH} -Wl,--rpath-link,${MIR_NDK_PATH}/lib -Wl,--rpath-link,${MIR_NDK_PATH}/lib/${MIR_ARM_EABI} -Wl,--rpath-link,${MIR_NDK_PATH}/usr/lib -Wl,--rpath-link,${MIR_NDK_PATH}/usr/lib/${MIR_ARM_EABI}")
 
 set(ENV{PKG_CONFIG_PATH} "${MIR_NDK_PATH}/usr/lib/pkgconfig:${MIR_NDK_PATH}/usr/lib/${MIR_ARM_EABI}/pkgconfig")
 set(ENV{PKG_CONFIG_SYSROOT_DIR} "${MIR_NDK_PATH}")

=== modified file 'cmake/MirCommon.cmake'
--- cmake/MirCommon.cmake	2014-08-25 03:11:01 +0000
+++ cmake/MirCommon.cmake	2014-08-27 02:19:21 +0000
@@ -39,10 +39,8 @@
   if(DISABLE_GTEST_TEST_DISCOVERY)
     add_test(${EXECUTABLE} ${VALGRIND_EXECUTABLE} ${VALGRIND_ARGS} ${EXECUTABLE_OUTPUT_PATH}/${EXECUTABLE} "--gtest_filter=-*DeathTest.*")
     add_test(${EXECUTABLE}_death_tests ${EXECUTABLE_OUTPUT_PATH}/${EXECUTABLE} "--gtest_filter=*DeathTest.*")
-    if (${ARGC} GREATER 1)
-      set_property(TEST ${EXECUTABLE} PROPERTY ENVIRONMENT ${ARGN})
-      set_property(TEST ${EXECUTABLE}_death_tests PROPERTY ENVIRONMENT ${ARGN})
-    endif()
+    set_property(TEST ${EXECUTABLE} PROPERTY ENVIRONMENT LD_LIBRARY_PATH=${LIBRARY_OUTPUT_PATH} ${ARGN})
+    set_property(TEST ${EXECUTABLE}_death_tests PROPERTY ENVIRONMENT LD_LIBRARY_PATH=${LIBRARY_OUTPUT_PATH} ${ARGN})
   else()
     set(CHECK_TEST_DISCOVERY_TARGET_NAME "check_discover_tests_in_${EXECUTABLE}")
     set(TEST_DISCOVERY_TARGET_NAME "discover_tests_in_${EXECUTABLE}")
@@ -51,7 +49,7 @@
     # These targets are always considered out-of-date, and are always run (at least for normal builds, except for make test/install).
     add_custom_target(
       ${CHECK_TEST_DISCOVERY_TARGET_NAME} ALL
-      ${EXECUTABLE_OUTPUT_PATH}/${EXECUTABLE} --gtest_list_tests > /dev/null
+      LD_LIBRARY_PATH=${LIBRARY_OUTPUT_PATH} ${EXECUTABLE_OUTPUT_PATH}/${EXECUTABLE} --gtest_list_tests > /dev/null
       WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
       COMMENT "Check that discovering Tests in ${EXECUTABLE} works")
       
@@ -63,6 +61,7 @@
       add_dependencies(${CHECK_TEST_DISCOVERY_TARGET_NAME} mirplatformgraphicsmesa)
     endif()
 
+    list(APPEND EXTRA_ENV_FLAGS "--add-environment" "LD_LIBRARY_PATH=${LIBRARY_OUTPUT_PATH}")
     if (${ARGC} GREATER 1)
       foreach (env ${ARGN})
         list(APPEND EXTRA_ENV_FLAGS "--add-environment" "${env}")
@@ -71,7 +70,7 @@
 
     add_custom_target(
       ${TEST_DISCOVERY_TARGET_NAME} ALL
-      ${EXECUTABLE_OUTPUT_PATH}/${EXECUTABLE} --gtest_list_tests | ${CMAKE_BINARY_DIR}/mir_gtest/mir_discover_gtest_tests --executable=${EXECUTABLE_OUTPUT_PATH}/${EXECUTABLE} ${DISCOVER_FLAGS}
+      LD_LIBRARY_PATH=${LIBRARY_OUTPUT_PATH} ${EXECUTABLE_OUTPUT_PATH}/${EXECUTABLE} --gtest_list_tests | ${CMAKE_BINARY_DIR}/mir_gtest/mir_discover_gtest_tests --executable=${EXECUTABLE_OUTPUT_PATH}/${EXECUTABLE} ${DISCOVER_FLAGS}
       ${EXTRA_ENV_FLAGS}
       WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
       COMMENT "Discovering Tests in ${EXECUTABLE}" VERBATIM)