Merge lp://staging/~compiz-team/compiz/compiz.fix_1058577 into lp://staging/compiz/0.9.9

Proposed by Sam Spilsbury
Status: Merged
Approved by: Daniel van Vugt
Approved revision: 3408
Merged at revision: 3413
Proposed branch: lp://staging/~compiz-team/compiz/compiz.fix_1058577
Merge into: lp://staging/compiz/0.9.9
Diff against target: 500 lines (+371/-26)
5 files modified
compizconfig/libcompizconfig/src/bindings.c (+52/-26)
compizconfig/libcompizconfig/src/ccs-modifier-list-inl.h (+50/-0)
compizconfig/libcompizconfig/src/ccs-private.h (+19/-0)
compizconfig/libcompizconfig/tests/CMakeLists.txt (+12/-0)
compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp (+238/-0)
To merge this branch: bzr merge lp://staging/~compiz-team/compiz/compiz.fix_1058577
Reviewer Review Type Date Requested Status
Daniel van Vugt Approve
jenkins (community) continuous-integration Needs Fixing
Sam Spilsbury Approve
Review via email: mp+128924@code.staging.launchpad.net

This proposal supersedes a proposal from 2012-10-10.

Commit message

Added tests for the StringToModifiers and ModifiersToString code, move the
code which detects <Primary> as ControlMask upstream and use two functions to
ensure that duplicates aren't added.

Fixes Jenkins failures. (LP: #1058577)

Description of the change

Added tests for the StringToModifiers and ModifiersToString code, move the code which detects <Primary> as ControlMask upstream and use two functions to ensure that duplicates aren't added.

Two tests were failing in bug 1058577 because both <Control> and <Primary> were added to the modifier string instead of just <Control>.

To post a comment you must log in.
Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Note, jenkins will probably mark this needs fixing because of conflicts with a distro patch. That distro patch should be dropped as its effectively made redundant by this branch.

Revision history for this message
jenkins (martin-mrazik+qa) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal
Download full text (5.3 KiB)

Sorry, but gcc sucks at template error messages:

[ 40%] Building CXX object compizconfig/libcompizconfig/tests/CMakeFiles/compizconfig_test_ccs_util.dir/compizconfig_test_ccs_util.cpp.o
In file included from /usr/include/c++/4.6/vector:70:0,
                 from /usr/include/gtest/gtest.h:55,
                 from /home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp:22:
/usr/include/c++/4.6/bits/vector.tcc: In member function ‘void std::vector<_Tp, _Alloc>::_M_insert_aux(std::vector<_Tp, _Alloc>::iterator, _Args&& ...) [with _Args = {{anonymous}::ModifierParam}, _Tp = {anonymous}::ModifierParam, _Alloc = std::allocator<{anonymous}::ModifierParam>, std::vector<_Tp, _Alloc>::iterator = __gnu_cxx::__normal_iterator<{anonymous}::ModifierParam*, std::vector<{anonymous}::ModifierParam> >, typename std::_Vector_base<_Tp, _Alloc>::_Tp_alloc_type::pointer = {anonymous}::ModifierParam*]’:
/usr/include/c++/4.6/bits/vector.tcc:102:4: instantiated from ‘void std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {{anonymous}::ModifierParam}, _Tp = {anonymous}::ModifierParam, _Alloc = std::allocator<{anonymous}::ModifierParam>]’
/usr/include/c++/4.6/bits/stl_vector.h:840:9: instantiated from ‘void std::vector<_Tp, _Alloc>::push_back(std::vector<_Tp, _Alloc>::value_type&&) [with _Tp = {anonymous}::ModifierParam, _Alloc = std::allocator<{anonymous}::ModifierParam>, std::vector<_Tp, _Alloc>::value_type = {anonymous}::ModifierParam]’
/home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp:131:22: instantiated from here
/usr/include/c++/4.6/bits/vector.tcc:319:4: error: use of deleted function ‘{anonymous}::ModifierParam& {anonymous}::ModifierParam::operator=(const {anonymous}::ModifierParam&)’
/home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp:94:11: error: ‘{anonymous}::ModifierParam& {anonymous}::ModifierParam::operator=(const {anonymous}::ModifierParam&)’ is implicitly deleted because the default definition would be ill-formed:
/home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp:94:11: error: passing ‘const string {aka const std::basic_string<char>}’ as ‘this’ argument of ‘std::basic_string<_CharT, _Traits, _Alloc>& std::basic_string<_CharT, _Traits, _Alloc>::operator=(const std::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char, _Traits = std::char_traits<char>, _Alloc = std::allocator<char>, std::basic_string<_CharT, _Traits, _Alloc> = std::basic_string<char>]’ discards qualifiers [-fpermissive]
In file included from /usr/include/c++/4.6/vector:61:0,
                 from /usr/include/gtest/gtest.h:55,
                 from /home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp:22:
/usr/include/c++/4.6/bits/stl_algobase.h: In static member function ‘static _BI2 std::__copy_move_backward<true, false, std::random_access_iterator_tag>::__copy_move_b(_BI1, _BI1, _BI2) [with _BI1 = {anonymous}::ModifierParam*, _BI2 = {anonymous}::ModifierParam*]’:
/usr/include/c++/4.6/bits/stl_algobase.h:581:18: instantiated from ‘_BI2 std::_...

Read more...

review: Needs Fixing
Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal
Download full text (5.6 KiB)

> Sorry, but gcc sucks at template error messages:
>
> [ 40%] Building CXX object compizconfig/libcompizconfig/tests/CMakeFiles/compi
> zconfig_test_ccs_util.dir/compizconfig_test_ccs_util.cpp.o
> In file included from /usr/include/c++/4.6/vector:70:0,
> from /usr/include/gtest/gtest.h:55,
> from /home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfi
> g/tests/compizconfig_test_ccs_util.cpp:22:
> /usr/include/c++/4.6/bits/vector.tcc: In member function ‘void
> std::vector<_Tp, _Alloc>::_M_insert_aux(std::vector<_Tp, _Alloc>::iterator,
> _Args&& ...) [with _Args = {{anonymous}::ModifierParam}, _Tp =
> {anonymous}::ModifierParam, _Alloc =
> std::allocator<{anonymous}::ModifierParam>, std::vector<_Tp, _Alloc>::iterator
> = __gnu_cxx::__normal_iterator<{anonymous}::ModifierParam*,
> std::vector<{anonymous}::ModifierParam> >, typename std::_Vector_base<_Tp,
> _Alloc>::_Tp_alloc_type::pointer = {anonymous}::ModifierParam*]’:
> /usr/include/c++/4.6/bits/vector.tcc:102:4: instantiated from ‘void
> std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args =
> {{anonymous}::ModifierParam}, _Tp = {anonymous}::ModifierParam, _Alloc =
> std::allocator<{anonymous}::ModifierParam>]’
> /usr/include/c++/4.6/bits/stl_vector.h:840:9: instantiated from ‘void
> std::vector<_Tp, _Alloc>::push_back(std::vector<_Tp, _Alloc>::value_type&&)
> [with _Tp = {anonymous}::ModifierParam, _Alloc =
> std::allocator<{anonymous}::ModifierParam>, std::vector<_Tp,
> _Alloc>::value_type = {anonymous}::ModifierParam]’
> /home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfig/tests/compizconfig_t
> est_ccs_util.cpp:131:22: instantiated from here
> /usr/include/c++/4.6/bits/vector.tcc:319:4: error: use of deleted function
> ‘{anonymous}::ModifierParam& {anonymous}::ModifierParam::operator=(const
> {anonymous}::ModifierParam&)’
> /home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfig/tests/compizconfig_t
> est_ccs_util.cpp:94:11: error: ‘{anonymous}::ModifierParam&
> {anonymous}::ModifierParam::operator=(const {anonymous}::ModifierParam&)’ is
> implicitly deleted because the default definition would be ill-formed:
> /home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfig/tests/compizconfig_t
> est_ccs_util.cpp:94:11: error: passing ‘const string {aka const
> std::basic_string<char>}’ as ‘this’ argument of ‘std::basic_string<_CharT,
> _Traits, _Alloc>& std::basic_string<_CharT, _Traits, _Alloc>::operator=(const
> std::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char, _Traits =
> std::char_traits<char>, _Alloc = std::allocator<char>,
> std::basic_string<_CharT, _Traits, _Alloc> = std::basic_string<char>]’
> discards qualifiers [-fpermissive]
> In file included from /usr/include/c++/4.6/vector:61:0,
> from /usr/include/gtest/gtest.h:55,
> from /home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfi
> g/tests/compizconfig_test_ccs_util.cpp:22:
> /usr/include/c++/4.6/bits/stl_algobase.h: In static member function ‘static
> _BI2 std::__copy_move_backward<true, false,
> std::random_access_iterator_tag>::__copy_move_b(_BI1, _BI1, _BI2) [with _BI1 =
> {anonymous}::ModifierParam*, _BI2 = {anony...

Read more...

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

And I tried clang to get better error messages. But hit bug 1060804 instead.

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

I cannot reproduce this compiler error but I've implemented copying semantics for the offending struct.

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

Seems OK now.

review: Approve
Revision history for this message
jenkins (martin-mrazik+qa) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Note that tarmac will probably reject this due to the distro patch

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/308/console reported an error when processing this lp:~compiz-team/compiz/compiz.fix_1058577 branch.
Not merging it.

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

I think jenkins is using the new packaging repo with the removed conflicting patch, resubmitting.

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

As above

review: Approve
Revision history for this message
jenkins (martin-mrazik+qa) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
jenkins (martin-mrazik+qa) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Sam Spilsbury (smspillaz) :
review: Approve
Revision history for this message
jenkins (martin-mrazik+qa) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Isn't this the same revision I approved already in a different submission?

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

P.S. Jenkins can't ever pass until the similar distro patch is removed from lp:ubuntu/compiz: primary_is_control.patch

Revision history for this message
jenkins (martin-mrazik+qa) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Right, although we've changed the packaging repo used for jenkins to lp:~compiz-team/compiz/ubuntu , although its still failing and the publisher is broken so we can't see why

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

OK, I have hacked away the offending reference to primary_is_control.patch in lp:~compiz-team/compiz/ubuntu

Try again.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches