Merge lp:~compiz-team/compiz/compiz.fix_python_tests_failing_in_chroots into lp:compiz/0.9.8

[Sam Spilsbury]

== Problem ==

Python tests fail non-determininstically in chroots

== Solution ==

  Fix some problems with python tests failing in chroots

  1) Don't allow duplicate symbols
  2) Set environment variables properly so that we always use the ini backend
  3) Always use the specified testing config file for compizconfig

== Test ==

Already covered by python tests

UNBLOCK

Resubmitting this to check if its still relevant.

[Daniel van Vugt (vanvugt)]

Works OK, but I'm a little concerned about dynamic linkage to locally built libraries in test cases:
add_library (gsettings_backend_shared SHARED

This means we're either relying on an LD_LIBRARY_PATH that is implicitly set (I don't see one) or the hardcoding of library paths (the source build directory) into our binaries.

How do we ensure tests find the right libgsettings_backend_shared.so at runtime?

review: Needs Information

[Sam Spilsbury (smspillaz)]

> Works OK, but I'm a little concerned about dynamic linkage to locally built
> libraries in test cases:
> add_library (gsettings_backend_shared SHARED
>
> This means we're either relying on an LD_LIBRARY_PATH that is implicitly set
> (I don't see one) or the hardcoding of library paths (the source build
> directory) into our binaries.
>
> How do we ensure tests find the right libgsettings_backend_shared.so at
> runtime?

The library is installed in the right place.

46 + install (TARGETS gsettings_backend_shared
47 + DESTINATION ${CMAKE_INSTALL_PREFIX}/lib)

[Daniel van Vugt (vanvugt)]

That's not relevant. Tests don't use the install tree. They use the build tree.

[Sam Spilsbury (smspillaz)]

> That's not relevant. Tests don't use the install tree. They use the build
> tree.

Oh ok.

In terms of the build tree, the right libraries are automatically pulled in at runtime, since libgsettings.so is linked to libgsettings_backend_shared.so (which resolves to the local version inside the build tree, and then is re-linked at install time).

Also from what I've read its not possible to change LD_LIBRARY_PATH at runtime anyways, you need to use a wrapper to do it, which isn't very valgrind friendly.

[Francis Ginther (fginther)]

Review was claimed by accident, please ignore.

review: Abstain

[Daniel van Vugt (vanvugt)]

OK, well it works for now. Just be aware that I think you're relying on an implicit -rpath (see: man ld) to find the shared library at test-time. That could change/break with different compilers in future.

It *might* be safe if ctest sets up LD_LIBRARY_PATH for you. I'm not sure if it does or if it's just working because of the hard-coded -rpath. But it works for now.

review: Approve

[Unity Merger (unity-merger)]

No commit message specified.

[Unity Merger (unity-merger)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-compiz-core/36/console reported an error when processing this lp:~compiz-team/compiz/compiz.fix_python_tests_failing_in_chroots branch.
Not merging it.

[Sam Spilsbury (smspillaz)]

^ packaging needs to be updated for this to work it seems.

[Daniel van Vugt (vanvugt)]

Added didrocks to look at the packaging. Is it still didrocks or someone else?

[Sam Spilsbury (smspillaz)]

subscribed sil2100, as he does the compiz packaging nowadays I think

[Daniel van Vugt (vanvugt)]

FYI, it's CMake setting the rpath that makes libgsettings_backend_shared.so findable at test time, within the build tree...
http://www.cmake.org/cmake/help/v2.8.8/cmake.html#command:set_target_properties

[Unity Merger (unity-merger)]

Attempt to merge into lp:compiz failed due to conflicts:

text conflict in compizconfig/compizconfig-python/tests/compiz_config_test.py

[Daniel van Vugt (vanvugt)]

Lots of conflicts.

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

I don't think we'll be needing this now, as most of it has been merged into lp:compiz in the form of other branches.

review: Disapprove

Unmerged revisions

3295. By Sam Spilsbury on 2012-07-27

Be more pythonic

3294. By Sam Spilsbury on 2012-07-25

Fix some problems with python tests failing in chroots

1) Don't allow duplicate symbols
2) Set environment variables properly so that we always use the ini backend
3) Always use the specified testing config file for compizconfig

=== modified file 'compizconfig/compizconfig-python/tests/compiz_config_test.py'
--- compizconfig/compizconfig-python/tests/compiz_config_test.py	2012-08-09 18:30:54 +0000
+++ compizconfig/compizconfig-python/tests/compiz_config_test.py	2012-08-22 13:19:51 +0000
@@ -1,9 +1,18 @@
 import os
+
+os.environ["G_SLICE"] = "always-malloc"
+os.environ["COMPIZ_CONFIG_PROFILE"] = ""
+os.environ["XDG_CONFIG_HOME"] = "compizconfig/libcompizconfig/config"
+os.environ["COMPIZ_METADATA_PATH"] = "generated/"
+os.environ["LIBCOMPIZCONFIG_BACKEND_PATH"] = "compizconfig/libcompizconfig/backend/"
+os.environ["XDG_DATA_DIRS"] = "generated/"
+
 import sys
 import subprocess
 
 arch = subprocess.Popen (["uname", "-p"], stdout=subprocess.PIPE).communicate ()[0][:-1]
 
+<<<<<<< TREE
 os.environ["G_SLICE"] = "always-malloc"
 os.environ["COMPIZ_METADATA_PATH"] = "generated/"
 os.environ["COMPIZ_CONFIG_PROFILE"] = ""
@@ -12,6 +21,9 @@
 os.environ["XDG_DATA_DIRS"] = "generated/"
 
 sys.path.insert (0, "compizconfig/compizconfig-python/build/lib.linux-%s-%s.%s/" % (arch, sys.version_info[0], sys.version_info[1]))
+=======
+sys.path.insert (0, "compizconfig/compizconfig-python/build/lib.linux-%s-%s.%s/" % (arch, sys.version_info[0], sys.version_info[1]))
+>>>>>>> MERGE-SOURCE
 
 import unittest
 import compizconfig

=== modified file 'compizconfig/gsettings/gsettings_backend_shared/CMakeLists.txt'
--- compizconfig/gsettings/gsettings_backend_shared/CMakeLists.txt	2012-08-19 10:02:23 +0000
+++ compizconfig/gsettings/gsettings_backend_shared/CMakeLists.txt	2012-08-22 13:19:51 +0000
@@ -15,6 +15,7 @@
     include_directories (${GSETTINGS_UTIL_LIB_INCLUDES})
 
     link_directories (${GSETTINGS_UTIL_LIBRARY_DIRS}
+<<<<<<< TREE
 		      ${compiz_BINARY_DIR}/compizconfig/libcompizconfig
                       ${compiz_BINARY_DIR}/compizconfig/integration/gnome
                       ${CMAKE_BINARY_DIR}/compizconfig/integration/gnome/gconf
@@ -76,5 +77,28 @@
     set_target_properties (compizconfig_gsettings_backend PROPERTIES
 	LINK_INTERFACE_LIBRARIES ""
     )
+=======
+		      ${compiz_BINARY_DIR}/compizconfig/libcompizconfig)
+
+    add_library (gsettings_backend_shared SHARED
+		 ${CMAKE_CURRENT_SOURCE_DIR}/gsettings_constants.c
+		 ${CMAKE_CURRENT_SOURCE_DIR}/gsettings_util.c)
+
+
+    target_link_libraries (gsettings_backend_shared
+			   ${GSETTINGS_UTIL_LIBRARIES}
+			   compizconfig)
+>>>>>>> MERGE-SOURCE
+
+    install (TARGETS gsettings_backend_shared
+	     DESTINATION ${CMAKE_INSTALL_PREFIX}/lib)
+
+    #
+    # Tell CMake that targets using gsettings_backend_shared should NOT re-import the
+    # libraries that gsettings_backend_shared depends on (contains).
+    #
+    set_target_properties (gsettings_backend_shared PROPERTIES
+	LINK_INTERFACE_LIBRARIES ""
+    )
 
 endif (GSETTINGS_UTIL_FOUND)

=== added file 'compizconfig/libcompizconfig/config/config_test'
--- compizconfig/libcompizconfig/config/config_test	1970-01-01 00:00:00 +0000
+++ compizconfig/libcompizconfig/config/config_test	2012-08-22 13:19:51 +0000
@@ -0,0 +1,3 @@
+[general]
+backend = ini
+plugin_list_autosort = true

=== renamed file 'compizconfig/libcompizconfig/config/config_test' => 'compizconfig/libcompizconfig/config/config_test.moved'
=== modified file 'compizconfig/libcompizconfig/src/CMakeLists.txt'
--- compizconfig/libcompizconfig/src/CMakeLists.txt	2012-07-26 23:11:25 +0000
+++ compizconfig/libcompizconfig/src/CMakeLists.txt	2012-08-22 13:19:51 +0000
@@ -68,6 +68,7 @@
     compizconfig ${LIBCOMPIZCONFIG_LIBRARIES} m
 )
 
+<<<<<<< TREE
 #
 # Tell CMake that targets using compizconfig should NOT re-import the
 # libraries that compizconfig depends on (contains).
@@ -76,6 +77,16 @@
 		       LINK_INTERFACE_LIBRARIES ""
 )
 
+=======
+#
+# Tell CMake that targets using compizconfig should NOT re-import the
+# libraries that compizconfig depends on (contains).
+#
+set_target_properties (compizconfig PROPERTIES
+    LINK_INTERFACE_LIBRARIES ""
+)
+
+>>>>>>> MERGE-SOURCE
 set (COMPIZCONFIG_LIBS "")
 foreach (_val ${LIBCOMPIZCONFIG_LDFLAGS})
     set (COMPIZCONFIG_LIBS "${COMPIZCONFIG_LIBS}${_val} ")