Mir

[regression] Client functions residing in libmircommon

Bug #1415321 reported by Daniel van Vugt
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mir
Fix Released
High
Alan Griffiths
Mir 0.13.0
mir (Ubuntu)
Fix Released
High
Unassigned

Bug Description

We seem to now have a bunch of client functions living in libmircommon:

$ nm -D lib/libmircommon.so | grep ' mir_'
0000000000010287 T mir_event_get_close_surface_event
000000000001012a T mir_event_get_input_event
0000000000010264 T mir_event_get_orientation_event
0000000000010241 T mir_event_get_prompt_session_event
000000000001021e T mir_event_get_resize_event
00000000000101fb T mir_event_get_surface_event
0000000000010105 T mir_event_get_type
000000000001038e T mir_event_ref
00000000000103c6 T mir_event_unref
000000000002dae2 T mir_input_event_get_device_id
000000000002dbea T mir_input_event_get_event_time
000000000002dcf0 T mir_input_event_get_key_input_event
000000000002e5b5 T mir_input_event_get_pointer_input_event
000000000002e003 T mir_input_event_get_touch_input_event
000000000002d9d8 T mir_input_event_get_type
000000000002ddc9 T mir_key_input_event_get_action
000000000002de1e T mir_key_input_event_get_key_code
000000000002dfab T mir_key_input_event_get_modifiers
000000000002de43 T mir_key_input_event_get_scan_code
0000000000010368 T mir_orientation_event_get_direction
000000000002e6bb T mir_pointer_input_event_get_action
000000000002e7d9 T mir_pointer_input_event_get_axis_value
000000000002e733 T mir_pointer_input_event_get_button_state
000000000002e68f T mir_pointer_input_event_get_modifiers
0000000000010342 T mir_prompt_session_event_get_state
000000000001031c T mir_resize_event_get_height
00000000000102f6 T mir_resize_event_get_width
00000000000102aa T mir_surface_event_get_attribute
00000000000102d0 T mir_surface_event_get_attribute_value
000000000002dfd7 T mir_touch_input_event_get_modifiers
000000000002e1df T mir_touch_input_event_get_touch_action
000000000002e453 T mir_touch_input_event_get_touch_axis_value
000000000002e0dd T mir_touch_input_event_get_touch_count
000000000002e0f7 T mir_touch_input_event_get_touch_id
000000000002e349 T mir_touch_input_event_get_touch_tooltype

I think this could cause serious risk to clients/toolkits that call them. As their symbol versions are no longer tied to the stable client ABI, but instead tied to the more volatile mircommon ABI.

See original description
Tags: regression

Related branches

lp:~vanvugt/mir/test-1415321
Alan Griffiths: Approve
Alexandros Frantzis (community): Approve
PS Jenkins bot (community): Approve (continuous-integration)
Diff: 24 lines (+17/-0)
1 file modified
examples/CMakeLists.txt (+17/-0)
lp:~alan-griffiths/mir/fix-1415321
PS Jenkins bot (community): Approve (continuous-integration)
Kevin DuBois (community): Approve
Daniel van Vugt: Abstain
Alberto Aguirre (community): Approve
Chris Halse Rogers: Abstain
Diff: 1010 lines (+300/-258)
28 files modified
debian/changelog (+1/-1)
debian/control (+2/-2)
debian/libmirclient-dev.install (+1/-0)
debian/libmircommon4.install (+1/-1)
examples/CMakeLists.txt (+10/-2)
include/client/mir_toolkit/cursors.h (+6/-0)
include/client/mir_toolkit/mir_client_library.h (+1/-0)
include/common/mir_toolkit/common.h (+0/-2)
src/client/CMakeLists.txt (+13/-2)
src/client/events/CMakeLists.txt (+2/-2)
src/client/input/CMakeLists.txt (+7/-13)
src/client/input/input_event.cpp (+1/-1)
src/client/mir_cursor_api.cpp (+17/-15)
src/client/symbols-debug.map (+0/-8)
src/client/symbols.map (+164/-24)
src/common/CMakeLists.txt (+2/-7)
src/common/sharedlibrary/CMakeLists.txt (+4/-1)
src/common/symbols.map (+48/-163)
src/common/udev/CMakeLists.txt (+8/-0)
src/include/common/mir/logging/input_timestamp.h (+1/-5)
src/server/CMakeLists.txt (+3/-1)
src/server/graphics/nested/CMakeLists.txt (+0/-4)
src/server/input/default_configuration.cpp (+2/-0)
tests/integration-tests/CMakeLists.txt (+1/-2)
tests/unit-tests/CMakeLists.txt (+2/-1)
tests/unit-tests/client/input/test_android_input_receiver.cpp (+1/-1)
tests/unit-tests/input/test_cursor_controller.cpp (+1/-0)
tests/unit-tests/input/test_xcursor_loader.cpp (+1/-0)
lp:ubuntu/wily-proposed/mir
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Hmm, if we explicitly avoid versioning these C symbols then we might be able to get away with proxying through libmirclient... That could avoid any direct dependency between clients and libmircommon.

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

Also see:

$ nm -u .mir_demo_client_fingerpaint-uninstalled | grep MIR_COMMON
                 U mir_event_get_input_event@@MIR_COMMON_3.1
                 U mir_event_get_type@@MIR_COMMON_3.1
                 U mir_input_event_get_pointer_input_event@@MIR_COMMON_3.1
                 U mir_input_event_get_touch_input_event@@MIR_COMMON_3.1
                 U mir_input_event_get_type@@MIR_COMMON_3.1
                 U mir_pointer_input_event_get_action@@MIR_COMMON_3
                 U mir_pointer_input_event_get_axis_value@@MIR_COMMON_3
                 U mir_pointer_input_event_get_button_state@@MIR_COMMON_3
                 U mir_touch_input_event_get_touch_action@@MIR_COMMON_3.1
                 U mir_touch_input_event_get_touch_axis_value@@MIR_COMMON_3.1
                 U mir_touch_input_event_get_touch_count@@MIR_COMMON_3.1

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

I think this is High. Because once we bump to MIR_COMMON_4, some clients will need to simultaneously have loaded:
   libmircommon.so.3
   libmircommon.so.4
which if that doesn't work (?), will become a serious compatibility problem.

Changed in mir:
importance: Undecided → High
description: updated
Daniel van Vugt (vanvugt)
description: updated
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Origin (regression introduced in Mir 0.10.0):

------------------------------------------------------------
revno: 2129 [merge]
fixes bug: https://launchpad.net/bugs/1311699
author: Robert Carr <email address hidden>
committer: Daniel van Vugt <email address hidden>
branch nick: dev
timestamp: Tue 2014-12-09 11:29:51 +0800
message:
  Introduce a transition path to MirEvent 2.0, henceforth MirInputEvent.

  Direct access to MirEvent::type is deprecated, and a new method
  mir_event_get_type is encouraged. For what was mir_event_type_key/motion
  a new type mir_event_type_input will be returned. Client code may call
  mir_event_get_input_event(...) to retreive a "MirInputEvent*" a.k.a.
  MirEvent 2.0.
------------------------------------------------------------

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

Although it's only an issue if anyone actually uses the new API. So we should hold them back for now.

Changed in mir:
status: New → Triaged
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

Fix committed into lp:mir at revision None, scheduled for release in mir, milestone 0.11.0

Changed in mir:
status: Triaged → Fix Committed
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

We have a (disabled) test but no fix yet

Changed in mir:
status: Fix Committed → Triaged
Daniel van Vugt (vanvugt)
Changed in mir:
milestone: none → 0.12.0
Kevin DuBois (kdub)
Changed in mir:
milestone: 0.12.0 → 0.13.0
Daniel van Vugt (vanvugt)
Changed in mir (Ubuntu):
importance: Undecided → High
status: New → Triaged
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

The problem's getting worse. We now have server code using these...
src/server/shell/abstract_shell.cpp

Alan Griffiths (alan-griffiths)
Changed in mir:
assignee: nobody → Alan Griffiths (alan-griffiths)
status: Triaged → In Progress
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

Fix committed into lp:mir at revision None, scheduled for release in mir, milestone 0.13.0

Changed in mir:
status: In Progress → Fix Committed
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Fix reverted in lp:mir rev 2409.

Changed in mir:
status: Fix Committed → Triaged
assignee: Alan Griffiths (alan-griffiths) → nobody
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

This bug can be treated as Medium priority, so long as we're careful to not bump the common ABI. Treat it like the client ABI for now.

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

My mistake. Revert reverted. The fix is re-committed (r2410).

Changed in mir:
assignee: nobody → Alan Griffiths (alan-griffiths)
status: Triaged → Fix Committed
Alan Griffiths (alan-griffiths)
Changed in mir:
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package mir - 0.13.1+15.10.20150520-0ubuntu1

---------------
mir (0.13.1+15.10.20150520-0ubuntu1) wily; urgency=medium

  [ Cemil Azizoglu ]
  * New upstream release 0.13.1 (https://launchpad.net/mir/+milestone/0.13.1)
    - ABI summary: No ABI break. Servers and clients do not need rebuilding.
      . Mirclient ABI unchanged at 8
      . Mircommon ABI unchanged at 4
      . Mirplatform ABI unchanged at 7
      . Mirserver ABI unchanged at 31
    - Bug fixes:
      . Can't load app purchase UI without a U1 account (LP: #1450377)
      . Crash because uncaught exception in mir::events::add_touch (LP: #1437357)

 -- CI Train Bot <email address hidden> Wed, 20 May 2015 21:20:15 +0000

Changed in mir (Ubuntu):
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.