Compiz

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

  1. compiz.fix_python_tests_failing_in_chroots
  2. Merge into 0.9.8
Proposed by Sam Spilsbury
Status: Rejected
Rejected by: Sam Spilsbury
Proposed branch: lp:~compiz-team/compiz/compiz.fix_python_tests_failing_in_chroots
Merge into: lp:compiz/0.9.8
Diff against target: 111 lines (+50/-0) (has conflicts)
4 files modified
compizconfig/compizconfig-python/tests/compiz_config_test.py (+12/-0)
compizconfig/gsettings/gsettings_backend_shared/CMakeLists.txt (+24/-0)
compizconfig/libcompizconfig/config/config_test (+3/-0)
compizconfig/libcompizconfig/src/CMakeLists.txt (+11/-0)
Text conflict in compizconfig/compizconfig-python/tests/compiz_config_test.py
Text conflict in compizconfig/gsettings/gsettings_backend_shared/CMakeLists.txt
Conflict adding file compizconfig/libcompizconfig/config/config_test.  Moved existing file to compizconfig/libcompizconfig/config/config_test.moved.
Text conflict in compizconfig/libcompizconfig/src/CMakeLists.txt
To merge this branch: bzr merge lp:~compiz-team/compiz/compiz.fix_python_tests_failing_in_chroots
Reviewer Review Type Date Requested Status
Sam Spilsbury Disapprove
Daniel van Vugt Needs Fixing
Łukasz Zemczak Pending
Francis Ginther Pending
Didier Roche-Tolomelli Pending
Review via email: mp+120764@code.launchpad.net

This proposal supersedes a proposal from 2012-08-01.

Description of the change

== 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.

To post a comment you must log in.
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

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
Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

> 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)

Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

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

Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

> 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.

Revision history for this message
Francis Ginther (fginther) wrote : Posted in a previous version of this proposal

Review was claimed by accident, please ignore.

review: Abstain
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

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
Revision history for this message
Unity Merger (unity-merger) wrote : Posted in a previous version of this proposal

No commit message specified.

Revision history for this message
Unity Merger (unity-merger) wrote : Posted in a previous version of this proposal

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.

Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

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

Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

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

Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

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

Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

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

Revision history for this message
Unity Merger (unity-merger) wrote : Posted in a previous version of this proposal

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

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

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

Lots of conflicts.

review: Needs Fixing
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

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

Be more pythonic

3294. By Sam Spilsbury

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
1import os1import os
2
3os.environ["G_SLICE"] = "always-malloc"
4os.environ["COMPIZ_CONFIG_PROFILE"] = ""
5os.environ["XDG_CONFIG_HOME"] = "compizconfig/libcompizconfig/config"
6os.environ["COMPIZ_METADATA_PATH"] = "generated/"
7os.environ["LIBCOMPIZCONFIG_BACKEND_PATH"] = "compizconfig/libcompizconfig/backend/"
8os.environ["XDG_DATA_DIRS"] = "generated/"
9
2import sys10import sys
3import subprocess11import subprocess
412
5arch = subprocess.Popen (["uname", "-p"], stdout=subprocess.PIPE).communicate ()[0][:-1]13arch = subprocess.Popen (["uname", "-p"], stdout=subprocess.PIPE).communicate ()[0][:-1]
614
15<<<<<<< TREE
7os.environ["G_SLICE"] = "always-malloc"16os.environ["G_SLICE"] = "always-malloc"
8os.environ["COMPIZ_METADATA_PATH"] = "generated/"17os.environ["COMPIZ_METADATA_PATH"] = "generated/"
9os.environ["COMPIZ_CONFIG_PROFILE"] = ""18os.environ["COMPIZ_CONFIG_PROFILE"] = ""
@@ -12,6 +21,9 @@
12os.environ["XDG_DATA_DIRS"] = "generated/"21os.environ["XDG_DATA_DIRS"] = "generated/"
1322
14sys.path.insert (0, "compizconfig/compizconfig-python/build/lib.linux-%s-%s.%s/" % (arch, sys.version_info[0], sys.version_info[1]))23sys.path.insert (0, "compizconfig/compizconfig-python/build/lib.linux-%s-%s.%s/" % (arch, sys.version_info[0], sys.version_info[1]))
24=======
25sys.path.insert (0, "compizconfig/compizconfig-python/build/lib.linux-%s-%s.%s/" % (arch, sys.version_info[0], sys.version_info[1]))
26>>>>>>> MERGE-SOURCE
1527
16import unittest28import unittest
17import compizconfig29import compizconfig
1830
=== 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 @@
15 include_directories (${GSETTINGS_UTIL_LIB_INCLUDES})15 include_directories (${GSETTINGS_UTIL_LIB_INCLUDES})
1616
17 link_directories (${GSETTINGS_UTIL_LIBRARY_DIRS}17 link_directories (${GSETTINGS_UTIL_LIBRARY_DIRS}
18<<<<<<< TREE
18 ${compiz_BINARY_DIR}/compizconfig/libcompizconfig19 ${compiz_BINARY_DIR}/compizconfig/libcompizconfig
19 ${compiz_BINARY_DIR}/compizconfig/integration/gnome20 ${compiz_BINARY_DIR}/compizconfig/integration/gnome
20 ${CMAKE_BINARY_DIR}/compizconfig/integration/gnome/gconf21 ${CMAKE_BINARY_DIR}/compizconfig/integration/gnome/gconf
@@ -76,5 +77,28 @@
76 set_target_properties (compizconfig_gsettings_backend PROPERTIES77 set_target_properties (compizconfig_gsettings_backend PROPERTIES
77 LINK_INTERFACE_LIBRARIES ""78 LINK_INTERFACE_LIBRARIES ""
78 )79 )
80=======
81 ${compiz_BINARY_DIR}/compizconfig/libcompizconfig)
82
83 add_library (gsettings_backend_shared SHARED
84 ${CMAKE_CURRENT_SOURCE_DIR}/gsettings_constants.c
85 ${CMAKE_CURRENT_SOURCE_DIR}/gsettings_util.c)
86
87
88 target_link_libraries (gsettings_backend_shared
89 ${GSETTINGS_UTIL_LIBRARIES}
90 compizconfig)
91>>>>>>> MERGE-SOURCE
92
93 install (TARGETS gsettings_backend_shared
94 DESTINATION ${CMAKE_INSTALL_PREFIX}/lib)
95
96 #
97 # Tell CMake that targets using gsettings_backend_shared should NOT re-import the
98 # libraries that gsettings_backend_shared depends on (contains).
99 #
100 set_target_properties (gsettings_backend_shared PROPERTIES
101 LINK_INTERFACE_LIBRARIES ""
102 )
79103
80endif (GSETTINGS_UTIL_FOUND)104endif (GSETTINGS_UTIL_FOUND)
81105
=== 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 @@
1[general]
2backend = ini
3plugin_list_autosort = true
04
=== 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 @@
68 compizconfig ${LIBCOMPIZCONFIG_LIBRARIES} m68 compizconfig ${LIBCOMPIZCONFIG_LIBRARIES} m
69)69)
7070
71<<<<<<< TREE
71#72#
72# Tell CMake that targets using compizconfig should NOT re-import the73# Tell CMake that targets using compizconfig should NOT re-import the
73# libraries that compizconfig depends on (contains).74# libraries that compizconfig depends on (contains).
@@ -76,6 +77,16 @@
76 LINK_INTERFACE_LIBRARIES ""77 LINK_INTERFACE_LIBRARIES ""
77)78)
7879
80=======
81#
82# Tell CMake that targets using compizconfig should NOT re-import the
83# libraries that compizconfig depends on (contains).
84#
85set_target_properties (compizconfig PROPERTIES
86 LINK_INTERFACE_LIBRARIES ""
87)
88
89>>>>>>> MERGE-SOURCE
79set (COMPIZCONFIG_LIBS "")90set (COMPIZCONFIG_LIBS "")
80foreach (_val ${LIBCOMPIZCONFIG_LDFLAGS})91foreach (_val ${LIBCOMPIZCONFIG_LDFLAGS})
81 set (COMPIZCONFIG_LIBS "${COMPIZCONFIG_LIBS}${_val} ")92 set (COMPIZCONFIG_LIBS "${COMPIZCONFIG_LIBS}${_val} ")

Subscribers

People subscribed via source and target branches