Mir

Merge lp:~andreas-pokorny/mir/bump-input-platform-abi-for-0.18 into lp:mir

Proposed by Andreas Pokorny on 2015-10-14
Status: Superseded
Proposed branch: lp:~andreas-pokorny/mir/bump-input-platform-abi-for-0.18
Merge into: lp:mir
Diff against target: 76 lines (+11/-8)
5 files modified
debian/control (+1/-1)
debian/mir-platform-input-evdev4.install (+1/-1)
src/platforms/CMakeLists.txt (+7/-4)
src/platforms/input_platform_symbols.map (+1/-1)
src/platforms/mesa/server/x11/symbols.map (+1/-1)
To merge this branch: bzr merge lp:~andreas-pokorny/mir/bump-input-platform-abi-for-0.18
Reviewer Review Type Date Requested Status
Daniel van Vugt Needs Fixing on 2015-10-23
PS Jenkins bot continuous-integration Needs Fixing on 2015-10-22
Alan Griffiths 2015-10-14 Needs Fixing on 2015-10-21
Review via email: mp+274410@code.launchpad.net

Commit Message

bump input platform abi and update our tools to work with the current guideline

Description of the Change

Upcoming changes like the improved modifier handling and the settings stuff break the abi

Please have a look at the updated regexs. I am not sure if \. or . is to be used in the various cases. It seemed to work in this case..

To post a comment you must log in.
Alan Griffiths (alan-griffiths) wrote :

We should introduce a way to use MIR_SERVER_INPUT_PLATFORM_ABI to generate the stanza name.

review: Approve
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
3022. By Alan Griffiths on 2015-10-14

tests, examples, 3rd_party: remove legacy mentions of xcursor support.

Approved by PS Jenkins bot, Andreas Pokorny, Kevin DuBois.

3023. By Alan Griffiths on 2015-10-14

mirtest-dev: add StubSession and StubSurface for use by downstream projects.

Approved by PS Jenkins bot, Andreas Pokorny, Kevin DuBois.

3024. By Cemil Azizoglu on 2015-10-14

Install a missed cookie header.

Approved by PS Jenkins bot, Brandon Schaefer.

3025. By Daniel van Vugt on 2015-10-15

Add a new acceptance test for the assertion that our weird 59Hz input
resampling actually reduces latency.
.

Approved by Kevin DuBois, Alan Griffiths, PS Jenkins bot.

PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
3026. By Alan Griffiths on 2015-10-15

examples: consolidate code in the window management examples.

Approved by Alexandros Frantzis, Andreas Pokorny, PS Jenkins bot.

3027. By Kevin DuBois on 2015-10-15

compositor: keep track of the number of buffers owned by client and server in mc::BufferMap.

Approved by PS Jenkins bot, Alexandros Frantzis, Andreas Pokorny, Alan Griffiths.

3028. By Kevin DuBois on 2015-10-15

client: add a MIR_CLIENT_NBUFFERS environment option (2 or 3) that adjusts the queue length for new semantic systems.

Approved by Alexandros Frantzis, Chris Halse Rogers, PS Jenkins bot.

3029. By Alexandros Frantzis on 2015-10-15

client: Fix mirclient.pc to privately require mircookie

This is needed because mirclient publishes headers that include mircookie headers. Fixes: https://bugs.launchpad.net/bugs/1506537.

Approved by PS Jenkins bot, Brandon Schaefer.

3030. By Daniel van Vugt on 2015-10-16

Merge final Ubuntu changelog 0.17.0 + 0.16.1 back from distro.
.

Approved by Alan Griffiths, PS Jenkins bot.

3031. By Cemil Azizoglu on 2015-10-16

Mark ThreadedDispatcherSignalTest.keeps_dispatching_after_signal_interruption as DISABLED.

Daniel van Vugt (vanvugt) wrote :

Indeed you appear to have confused your back-quotes. Please use:

. --> match any character
\. --> match the '.' character
+ --> match one or more of the previous expression
\+ --> match the '+' character

Remember it's the shell you need to protect against, as the shell may interpret/convert special characters before your command receives the regex.

The only conflicting characters between the shell and a regex are:
   []
   *
   ?
So those need to be quoted on the command line.

However, if a string is seen by two shell commands like:
   foo="\.\.\."
   bar="$foo"
then you may need the backslash.

If you can't figure it out, just test and use what works. But I'm fairly sure there are mistakes right now, like:
   grep -o '[\.[:digit:]]\+'
should be:
   grep -o '[.[:digit:]]+'
or more precisely:
   grep -o '[.0-9]+'

review: Needs Fixing
3032. By Alan Griffiths on 2015-10-19

tests: workaround for racy teardown of test case. (Which tears down drivers). Fixes: https://bugs.launchpad.net/bugs/1502782.

Approved by PS Jenkins bot, Chris Halse Rogers, Kevin DuBois, Daniel van Vugt, Cemil Azizoglu.

3033. By Chris Halse Rogers on 2015-10-19

Add mir_demo_client_multistream.

This tests the mir_surface_spec_set_streams functionality we've had for a while, but didn't have any example client for.

Approved by Alexandros Frantzis, PS Jenkins bot, Andreas Pokorny, Kevin DuBois.

3034. By Daniel van Vugt on 2015-10-19

Fingerpaint: Use the actual touch size when painting. But also provide
a command line option to override it.

This provides better touch device support, and also acts as a workaround
for both LP: #1354254 and LP: #1487366 so fingerpaint is now usable under
Unity8 (if you remember -w to workaround LP: #1497828 too). Fixes: https://bugs.launchpad.net/bugs/1354254, https://bugs.launchpad.net/bugs/1487366.

Approved by PS Jenkins bot, Kevin DuBois, Andreas Pokorny.

3035. By Daniel van Vugt on 2015-10-19

Revert optimization r2782. Although it gave us a nice 5ms reduction in
touch latency, it also reduced Android-input's ability to smooth the curve
of the resulting event stream. And that reduction in smoothness is going
to be visible in some touch scrolling scenarios, just as it is visible in
mir_demo_client_fingerpaint / mir_demo_client_target. (LP: #1506331). Fixes: https://bugs.launchpad.net/bugs/1506331.

Approved by Alan Griffiths, PS Jenkins bot, Alexandros Frantzis.

3036. By Kevin DuBois on 2015-10-19

compositor: Stitch together the new buffer semantics bits to get ~40 tests from the BufferScheduling integration test suit.

Approved by Alexandros Frantzis, Chris Halse Rogers, PS Jenkins bot.

3037. By Andreas Pokorny on 2015-10-20

Pointer settings in libinput

Allows configuring cursor acceleration, scroll speed and left-hand/right-hand button ordering for all pointing devices. Fixes: https://bugs.launchpad.net/bugs/124440.

Approved by PS Jenkins bot, Chris Halse Rogers, Alan Griffiths.

3038. By Andreas Pokorny on 2015-10-20

Delay mesa hack: reloading of module only needed for nested mesa platform

The mesa hack has horrible side effects - on krillin it makes mtks mali-driver frequently segfault during module unload. Fixes: https://bugs.launchpad.net/bugs/1506137.

Approved by PS Jenkins bot, Kevin DuBois, Alan Griffiths.

3039. By Chris Halse Rogers on 2015-10-20

Handle surface streams configuration entirely in Mir.

A client surface with multiple buffer streams is still logically a single window. There's nothing for the WindowManager to do here; rejecting the request is not allowed by the protocol.
.

Approved by PS Jenkins bot, Kevin DuBois, Alexandros Frantzis.

3040. By Kevin DuBois on 2015-10-20

compositor: bring the resizing tests for the new semantics into the fold of the passing BufferScheduling tests.

.

Approved by Alexandros Frantzis, PS Jenkins bot, Chris Halse Rogers.

3041. By Kevin DuBois on 2015-10-20

compositor: BufferScheduling test: remove requirement that all buffers are cycled through by the client in swapinterval 0 scenarios.
.

Approved by PS Jenkins bot, Alexandros Frantzis, Chris Halse Rogers.

Daniel van Vugt (vanvugt) wrote :

(1) Needs fixing: I think \+ should mostly be +
Because there's no enclosing shell/interpreter you need to protect it from. And you need the regex to receive '+' and not '\+' which mean very different things.

(2) Needs information: Had we agreed to rename binary sonames already ("3" -> "0.18"). I'm not against the idea but thought there was some opposition to it when last discussed?

review: Needs Fixing
Daniel van Vugt (vanvugt) wrote :

*(1) Because the shell does not interpret '+' at all. You don't want the regex receiving '\+' when you mean '+'.

review: Needs Fixing
3042. By Daniel van Vugt on 2015-10-21

Welcome to series 0.18.

Approved by PS Jenkins bot.

Andreas Pokorny (andreas-pokorny) wrote :

(1) You might very will think that... but + would be wrong...

(2) I brought this up in stand up, and nobody opposed using 0.18.

3043. By Brandon Schaefer on 2015-10-21

Add a comment explaining the need for this temp variable.

Approved by Daniel van Vugt, PS Jenkins bot, Kevin DuBois.

Alan Griffiths (alan-griffiths) wrote :

Seems to work (which is more than Daniel's suggestion).

But we probably want to also update greater_soname_version() (in src/common/sharedlibrary/shared_library_prober.cpp) to understand this change to soname versioning.

review: Needs Fixing
Daniel van Vugt (vanvugt) wrote :

(1) For example, an "ABI number" does not usually contain the '+' character. It is one or more of a digit or dot. So '\+' needs to be '+'.

 get_abi_number()
 {
     local abi_var=$1
- grep -hR --include=CMakeLists.txt "set($abi_var [[:digit:]]\+)" src/ | grep -o '[[:digit:]]\+'
+ grep -hR --include=CMakeLists.txt "set($abi_var [.0-9]\+)" src/ | grep -o '[.0-9]\+'
 }

Please replace the instances of \+ with + or explain why that is wrong...

review: Needs Fixing
Daniel van Vugt (vanvugt) wrote :

Specifically:
+ grep -hR --include=CMakeLists.txt "set($abi_var [.0-9]\+)" src/ | grep -o '[.0-9]\+'
should be something like:
+ egrep -hR --include=CMakeLists.txt "set($abi_var [.0-9]+)" src/ | egrep -o '[.0-9]+'

And:
+ sed -i "s/${pkg}[.0-9]\+/${pkg}$abi/" debian/control
should be something like:
+ sed -i "s/${pkg}[.0-9]+/${pkg}$abi/" debian/control

And:
+ ls -1 debian/${pkg}*.install | grep -o "${pkg}[.0-9]\+.install"
should be something like:
+ ls -1 debian/${pkg}*.install | egrep -o "${pkg}[.0-9]+\.install"

And:
+ local result="$(grep -o "${pkg}[.0-9]\+" debian/control | sort | uniq | sed -e "/\b${pkg}${abi}\b/ d" | tr '\n' ' ')"
should be something like:
+ local result="$(egrep -o "${pkg}[.0-9]+" debian/control | sort | uniq | sed -e "/\b${pkg}${abi}\b/ d" | tr '\n' ' ')"

And:
+ local suffix=$(grep -o ".so.[.0-9]\+" $expected_file)
should be something like:
+ local suffix=$(egrep -o "\.so\.[.0-9]+" $expected_file)

Your expressions may be partially working by accident right now, because \+ is sometimes converted to + in the shell, and the regex '.' will match the literal '.' (but also any other character). However that does not make them correct. Admittedly much of the issues are pre-existing.

review: Needs Fixing
3044. By Chris Halse Rogers on 2015-10-22

Add dependency on libmircookie-dev to libmirclient-dev

We expose types from mircookie in mirclient, and we've (correctly) got a Requires.private in the pkgconfig file, so we also need a Depends:.

Approved by PS Jenkins bot, Daniel van Vugt, Alan Griffiths, Brandon Schaefer.

3045. By Brandon Schaefer on 2015-10-22

Add tests for mouse to test we dont get a mac when doing motion and we do get a mac when we click.

Approved by PS Jenkins bot, Alan Griffiths, Kevin DuBois, Chris Halse Rogers.

3046. By Kevin DuBois on 2015-10-22

correct client side problem where the created-with pixel format was overwritten in the new semantics.

Approved by PS Jenkins bot, Daniel van Vugt, Alan Griffiths.

3047. By Cemil Azizoglu on 2015-10-22

[mir-on-x, gtk-mir] Convert X11 Button4 and Button5 ButtonPress events as mir vscroll events.

Approved by PS Jenkins bot, Andreas Pokorny, Alan Griffiths, Alexandros Frantzis, Kevin DuBois.

Andreas Pokorny (andreas-pokorny) wrote :

Yes when switching to egrep the \+ has to become +. But this does not apply for sed. Additionally in certain invocations egrep does not support plain '+'.

> Specifically:
> + grep -hR --include=CMakeLists.txt "set($abi_var [.0-9]\+)" src/ | grep -o
> '[.0-9]\+'

Assume abi_ver == MIR_SERVER_INPUT_PLATFORM_ABI then the above yields
3

> should be something like:
> + egrep -hR --include=CMakeLists.txt "set($abi_var [.0-9]+)" src/ | egrep
> -o '[.0-9]+'

While the above here yields:

Maybe a side effect of the other parameters, since in other scenarios it is able to switch to ERE mode.

> And:
> + sed -i "s/${pkg}[.0-9]\+/${pkg}$abi/" debian/control

assume ${pkg}=mir-platform-input-evdev $abi=99 with the above it results into a replacement

> should be something like:
> + sed -i "s/${pkg}[.0-9]+/${pkg}$abi/" debian/control

The above will not replace anything, since sed also interpets '+' as a literal character.

>
> And:
> + ls -1 debian/${pkg}*.install | grep -o "${pkg}[.0-9]\+.install"
> should be something like:
> + ls -1 debian/${pkg}*.install | egrep -o "${pkg}[.0-9]+\.install"

The differentiation between \. and . is ok-isch but not relevant since ls already filters '.install' only files. But here both work. Since in that case grep uses BRE, and egrep ERE.

>
> And:
> + local result="$(grep -o "${pkg}[.0-9]\+" debian/control | sort | uniq
> | sed -e "/\b${pkg}${abi}\b/ d" | tr '\n' ' ')"
> should be something like:
> + local result="$(egrep -o "${pkg}[.0-9]+" debian/control | sort | uniq
> | sed -e "/\b${pkg}${abi}\b/ d" | tr '\n' ' ')"

Like above, both work. grep and egrep

>
> And:
> + local suffix=$(grep -o ".so.[.0-9]\+" $expected_file)
> should be something like:
> + local suffix=$(egrep -o "\.so\.[.0-9]+" $expected_file)
>
> Your expressions may be partially working by accident right now, because \+ is
> sometimes converted to + in the shell, and the regex '.' will match the
> literal '.' (but also any other character). However that does not make them
> correct. Admittedly much of the issues are pre-existing.

Not sure if that was your intention, but because of the inconsistencies above, I would refrain from switching to egrep, even for those cases where it would work.

3048. By Kevin DuBois on 2015-10-22

compositor: fix nbs test by correcting a scenario where the MultiMonitorArbiter would hold onto a second buffer for too long.

Approved by PS Jenkins bot, Alexandros Frantzis, Chris Halse Rogers.

Daniel van Vugt (vanvugt) wrote :

+#include <boost/spirit/home/x3/

Doesn't build, but also please don't add unnecessary new build-deps. Ubuntu already comes with at least two good regex parsers you can use. Although I suspect simply "sscanf()" might work even better :)

review: Needs Fixing
Daniel van Vugt (vanvugt) wrote :

Here's a simple parser that works (for what I think you want):

void get_version(const char *name, std::vector<unsigned int>& version)
{
    version.clear();
    while (*name)
    {
        unsigned int v;
        int len;
        if (sscanf(name, ".%u%n", &v, &len) == 1)
        {
            version.push_back(v);
            name += len;
        }
        else
        {
            ++name;
        }
    }
}

No spirit/x3 stuff required.

3049. By Andreas Pokorny on 2015-10-23

Untangle stanza name from ABI numbering

As of the current doc/dso_versioning_guide the stanza version suffix is changed to contain the
version the symbol was introduced (or last compatible mir version). To avoid confusing stanza
names with abi numbering this change separates this into two cmake variables, such that our scripting
for abi dumps and update_package_abis does not require changes.

3050. By Andreas Pokorny on 2015-10-26

configure stanza for input platforms from cmake

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/control'
2--- debian/control 2015-10-21 02:02:14 +0000
3+++ debian/control 2015-10-23 07:45:00 +0000
4@@ -332,7 +332,7 @@
5 Contains the shared libraries required for the Mir server to interact with
6 the hardware platform using the Android drivers.
7
8-Package: mir-platform-input-evdev3
9+Package: mir-platform-input-evdev4
10 Section: libs
11 Architecture: linux-any
12 Multi-Arch: same
13
14=== renamed file 'debian/mir-platform-input-evdev3.install' => 'debian/mir-platform-input-evdev4.install'
15--- debian/mir-platform-input-evdev3.install 2015-09-23 18:55:58 +0000
16+++ debian/mir-platform-input-evdev4.install 2015-10-23 07:45:00 +0000
17@@ -1,1 +1,1 @@
18-usr/lib/*/mir/server-platform/input-evdev.so.3
19+usr/lib/*/mir/server-platform/input-evdev.so.4
20
21=== modified file 'src/platforms/CMakeLists.txt'
22--- src/platforms/CMakeLists.txt 2015-10-05 02:18:18 +0000
23+++ src/platforms/CMakeLists.txt 2015-10-23 07:45:00 +0000
24@@ -3,13 +3,15 @@
25 # This ABI is much smaller than the full libmirplatform ABI.
26 #
27 # TODO: Add an extra driver-ABI check target.
28-set(MIR_SERVER_INPUT_PLATFORM_ABI 3)
29+set(MIR_SERVER_INPUT_PLATFORM_ABI 4)
30+set(MIR_SERVER_INPUT_PLATFORM_STANZA_VERSION 0.18)
31 set(MIR_SERVER_INPUT_PLATFORM_ABI ${MIR_SERVER_INPUT_PLATFORM_ABI} PARENT_SCOPE)
32-set(MIR_SERVER_INPUT_PLATFORM_VERSION "MIR_INPUT_PLATFORM_${MIR_SERVER_INPUT_PLATFORM_ABI}")
33+set(MIR_SERVER_INPUT_PLATFORM_VERSION "MIR_INPUT_PLATFORM_${MIR_SERVER_INPUT_PLATFORM_STANZA_VERSION}")
34 set(MIR_SERVER_INPUT_PLATFORM_VERSION ${MIR_SERVER_INPUT_PLATFORM_VERSION} PARENT_SCOPE)
35 set(MIR_SERVER_GRAPHICS_PLATFORM_ABI 6)
36+set(MIR_SERVER_GRAPHICS_PLATFORM_STANZA_VERSION 6)
37 set(MIR_SERVER_GRAPHICS_PLATFORM_ABI ${MIR_SERVER_GRAPHICS_PLATFORM_ABI} PARENT_SCOPE)
38-set(MIR_SERVER_GRAPHICS_PLATFORM_VERSION "MIR_GRAPHICS_PLATFORM_${MIR_SERVER_GRAPHICS_PLATFORM_ABI}")
39+set(MIR_SERVER_GRAPHICS_PLATFORM_VERSION "MIR_GRAPHICS_PLATFORM_${MIR_SERVER_GRAPHICS_PLATFORM_STANZA_VERSION}")
40 set(MIR_SERVER_GRAPHICS_PLATFORM_VERSION ${MIR_SERVER_GRAPHICS_PLATFORM_VERSION} PARENT_SCOPE)
41 set(MIR_INPUT_PLATFORM_VERSION_SCRIPT ${CMAKE_CURRENT_SOURCE_DIR}/input_platform_symbols.map)
42 set(MIR_INPUT_PLATFORM_VERSION_SCRIPT ${MIR_INPUT_PLATFORM_VERSION_SCRIPT} PARENT_SCOPE)
43@@ -23,8 +25,9 @@
44 )
45
46 set(MIR_CLIENT_PLATFORM_ABI 3)
47+set(MIR_CLIENT_PLATFORM_STANZA_VERSION 3)
48 set(MIR_CLIENT_PLATFORM_ABI ${MIR_CLIENT_PLATFORM_ABI} PARENT_SCOPE)
49-set(MIR_CLIENT_PLATFORM_VERSION "MIR_CLIENT_PLATFORM_${MIR_CLIENT_PLATFORM_ABI}")
50+set(MIR_CLIENT_PLATFORM_VERSION "MIR_CLIENT_PLATFORM_${MIR_CLIENT_PLATFORM_STANZA_VERSION}")
51 set(MIR_CLIENT_PLATFORM_VERSION ${MIR_CLIENT_PLATFORM_VERSION} PARENT_SCOPE)
52
53 set(server_common_include_dirs
54
55=== modified file 'src/platforms/input_platform_symbols.map'
56--- src/platforms/input_platform_symbols.map 2015-09-23 19:25:26 +0000
57+++ src/platforms/input_platform_symbols.map 2015-10-23 07:45:00 +0000
58@@ -1,4 +1,4 @@
59-MIR_INPUT_PLATFORM_3 {
60+MIR_INPUT_PLATFORM_0.18 {
61 global:
62 add_input_platform_options;
63 create_input_platform;
64
65=== modified file 'src/platforms/mesa/server/x11/symbols.map'
66--- src/platforms/mesa/server/x11/symbols.map 2015-09-29 13:41:51 +0000
67+++ src/platforms/mesa/server/x11/symbols.map 2015-10-23 07:45:00 +0000
68@@ -8,7 +8,7 @@
69 local: *;
70 };
71
72-MIR_INPUT_PLATFORM_3 {
73+MIR_INPUT_PLATFORM_0.18 {
74 global:
75 add_input_platform_options;
76 create_input_platform;

Subscribers

People subscribed via source and target branches