Mir

Merge lp://qastaging/~alan-griffiths/mir/fix-1617865 into lp://qastaging/mir

Proposed by Alan Griffiths
Status: Rejected
Rejected by: Alan Griffiths
Proposed branch: lp://qastaging/~alan-griffiths/mir/fix-1617865
Merge into: lp://qastaging/mir
Diff against target: 72 lines (+43/-0)
1 file modified
src/common/dispatch/readable_fd.cpp (+43/-0)
To merge this branch: bzr merge lp://qastaging/~alan-griffiths/mir/fix-1617865
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Approve
Alan Griffiths Disapprove
Brandon Schaefer (community) Approve
Review via email: mp+306123@code.qastaging.launchpad.net

Commit message

Fix the accidental move of mir::dispatch::ReadableFd symbols from MIR_COMMON_5.1 to MIR_COMMON_0.24 (LP:1617865)

To post a comment you must log in.
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

+1

review: Approve
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Ooh! This might be a problem...

16:30:48 cd /<<BUILDDIR>>/mir-0.25.0+xenial2257bzr3711/obj-x86_64-linux-gnu && /usr/bin/cmake -E cmake_depends "Unix Makefiles" /<<BUILDDIR>>/mir-0.25.0+xenial2257bzr3711 /<<BUILDDIR>>/mir-0.25.0+xenial2257bzr3711/tests/mir_test_doubles /<<BUILDDIR>>/mir-0.25.0+xenial2257bzr3711/obj-x86_64-linux-gnu /<<BUILDDIR>>/mir-0.25.0+xenial2257bzr3711/obj-x86_64-linux-gnu/tests/mir_test_doubles /<<BUILDDIR>>/mir-0.25.0+xenial2257bzr3711/obj-x86_64-linux-gnu/tests/mir_test_doubles/CMakeFiles/mir-test-doubles-platform-static.dir/DependInfo.cmake --color=
16:30:48 [ 48%] Built target mirclientrpc
16:30:48 make -f examples/CMakeFiles/mir_demo_server.dir/build.make examples/CMakeFiles/mir_demo_server.dir/depend
16:30:48 /tmp/cchcbiRQ.s: Assembler messages:
16:30:48 /tmp/cchcbiRQ.s:345: Error: multiple versions [`_ZN3mir8dispatch10ReadableFdD0Ev@@@MIR_COMMON_5.1'|`_ZN3mir8dispatch10ReadableFdD0Ev@MIR_COMMON_0.24'] for symbol `_ZN3mir8dispatch10ReadableFdD0Ev'
16:30:48 /tmp/cchcbiRQ.s:346: Error: multiple versions [`_ZTVN3mir8dispatch10ReadableFdE@@@MIR_COMMON_5.1'|`_ZTVN3mir8dispatch10ReadableFdE@MIR_COMMON_0.24'] for symbol `_ZTVN3mir8dispatch10ReadableFdE'
16:30:48 /tmp/cchcbiRQ.s:347: Error: multiple versions [`_ZN3mir8dispatch10ReadableFdC2ENS_2FdERKSt8functionIFvvEE@@@MIR_COMMON_5.1'|`_ZN3mir8dispatch10ReadableFdC2ENS_2FdERKSt8functionIFvvEE@MIR_COMMON_0.24'] for symbol `_ZN3mir8dispatch10ReadableFdC2ENS_2FdERKSt8functionIFvvEE'
16:30:48 /tmp/cchcbiRQ.s:773: Error: multiple versions [`_ZNK3mir8dispatch10ReadableFd8watch_fdEv@@@MIR_COMMON_5.1'|`_ZNK3mir8dispatch10ReadableFd8watch_fdEv@MIR_COMMON_0.24'] for symbol `_ZNK3mir8dispatch10ReadableFd8watch_fdEv'
16:30:48 /tmp/cchcbiRQ.s:806: Error: multiple versions [`_ZN3mir8dispatch10ReadableFd8dispatchEj@@@MIR_COMMON_5.1'|`_ZN3mir8dispatch10ReadableFd8dispatchEj@MIR_COMMON_0.24'] for symbol `_ZN3mir8dispatch10ReadableFd8dispatchEj'
16:30:48 /tmp/cchcbiRQ.s:854: Error: multiple versions [`_ZNK3mir8dispatch10ReadableFd15relevant_eventsEv@@@MIR_COMMON_5.1'|`_ZNK3mir8dispatch10ReadableFd15relevant_eventsEv@MIR_COMMON_0.24'] for symbol `_ZNK3mir8dispatch10ReadableFd15relevant_eventsEv'
16:30:48 src/common/CMakeFiles/mircommon.dir/build.make:113: recipe for target 'src/common/CMakeFiles/mircommon.dir/dispatch/readable_fd.cpp.o' failed
16:30:48 make[3]: *** [src/common/CMakeFiles/mircommon.dir/dispatch/readable_fd.cpp.o] Error 1
16:30:48 make[3]: Leaving directory '/<<BUILDDIR>>/mir-0.25.0+xenial2257bzr3711/obj-x86_64-linux-gnu'
16:30:48 CMakeFiles/Makefile2:2791: recipe for target 'src/common/CMakeFiles/mircommon.dir/all' failed
16:30:48 make[2]: *** [src/common/CMakeFiles/mircommon.dir/all] Error 2
16:30:48 make[2]: *** Waiting for unfinished jobs....

https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2231/console

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> Ooh! This might be a problem...
>
It is. clang "does the right thing" but gcc doesn't understand this approach.

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

I wish you luck.

Isn't @@@ meant to be just @@ ?

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> I wish you luck.
>
> Isn't @@@ meant to be just @@ ?

I thought so too. But I found the symbol wasn't exported with that and needed to be @@@ - like the example in doc/dso_versioning_guide.md.

What I overlooked was that I was using clang - which behaves differently to gcc. I need to RTFM.

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Hmm...

    Honestly, this is an assembler bug, but we've been working around
    it for years. Here you have to jump though silly hoops, and create
    an alternate base symbol via an alias. E.g.

    weak_alias (__mcount, __mcount1)
    versioned_symbol (libc, __mcount, _mcount, GLIBC_2_18)
    versioned_symbol (libc, __mcount1, mcount, GLIBC_2_18)

    There are other examples in the source base for similar if you go
    grepping...

https://sourceware.org/ml/libc-alpha/2013-07/msg00480.html

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3712
https://mir-jenkins.ubuntu.com/job/mir-ci/1771/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2219
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2282
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2273
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2273
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2273
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2247
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2247/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2247
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2247/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2247
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2247/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2247
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2247/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2247
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2247/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1771/rebuild

review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

This is weird. After the gcc assembler complaining about them in the last iteration it is now failing to emit the versioned symbols.

This seems to be specific to C++ symbols - I tried cut & paste of some incantations from Mir-0.13 and that works.

Maybe this approach only works with clang?

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Showing it to CI, but the gcc version still doesn't publish the versioned aliases from libmircommon.

review: Disapprove
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3715
https://mir-jenkins.ubuntu.com/job/mir-ci/1773/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2221
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2284
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2275
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2275
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2275
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2250
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2250/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2250
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2250/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2250
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2250/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2250
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2250/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2250
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2250/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1773/rebuild

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

Maybe symbols.map is conflicting/overriding this...

Maybe also consider using two separate compilation units (cpp files) with just a different symbol version in each.

3716. By Alan Griffiths

Code cleanup

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

The problem appear to be that for C++ symbols the gcc assembler exports the unversioned symbols as well as the versioned ones whereas the clang assembler just exports the versioned aliases.

With gcc the linker then resolves to the unversioned symbols (and ignores the versioned ones).

Marking the unversioned symbol names .local doesn't help.

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3716
https://mir-jenkins.ubuntu.com/job/mir-ci/1780/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2230
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2293
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2284
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2284
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2284
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2258
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2258/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2258
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2258/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2258
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2258/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2258
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2258/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2258
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2258/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1780/rebuild

review: Approve (continuous-integration)

Unmerged revisions

3716. By Alan Griffiths

Code cleanup

3715. By Alan Griffiths

Incantation for the vtab

3714. By Alan Griffiths

Delete test code

3713. By Alan Griffiths

Get the versioned aliases into the .o. (Shame about the .so)

3712. By Alan Griffiths

Work around undocumented behaviour of gcc and clang toolchains

3711. By Alan Griffiths

Provide mir::dispatch::ReadableFd symbols in both MIR_COMMON_5.1 and MIR_COMMON_0.24

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