Merge lp:~thomas-voss/mir/explicit-gcc-version into lp:mir

[Thomas Voß]

Explicitly select g++-4.9 to prevent from ABI breaks. (LP: #1329089)

Explicitly select g++-4.9 to prevent from ABI breaks. (LP: #1329089)

[Alan Griffiths (alan-griffiths)]

Lots of unrelated change. (Looks like development-branch may be the intended target.)

review: Needs Resubmitting

[Colin Watson (cjwatson)]

Bumping the version from 0.2.0-0ubuntu1 to 0.2.0-0ubuntu2 was unnecessary as 0.2.0-0ubuntu1 was marked UNRELEASED, so you could change that back.

No SONAME change required? I assume you've checked whether mir's ABI is affected. Also assume you've double-checked that cmake picks up CC and CXX from its environment.

The include /usr/share/dpkg/default.mk is unnecessary, since DEB_HOST_ARCH was already defined above.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1728
http://jenkins.qa.ubuntu.com/job/mir-ci/1666/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/728/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/734
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-utopic-amd64-ci/4/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-utopic-armhf-ci/4
        deb: http://jenkins.qa.ubuntu.com/job/mir-utopic-armhf-ci/4/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/1666/rebuild

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

Are we dropping trusty?

review: Needs Information

[Colin Watson (cjwatson)]

On Fri, Jun 27, 2014 at 11:38:48AM -0000, Alan Griffiths wrote:
> Are we dropping trusty?

Can't you build trusty from a different branch with a tweaked build-dep?

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1728
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~thomas-voss/mir/explicit-gcc-version/+merge/224787/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2019/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/729
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/735
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/728
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/540/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/538
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/538/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2183
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2183/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1892
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/8931

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2019/rebuild

review: Needs Fixing (continuous-integration)

[Alexander Sack (asac)]

Alan: why do you need trusty to be supported on trunk? Any insight on the requirements and rational will be helpful here.

[Alan Griffiths (alan-griffiths)]

> Alan: why do you need trusty to be supported on trunk? Any insight on the
> requirements and rational will be helpful here.

I was just asking as it currently works.

review: Abstain

[Alan Griffiths (alan-griffiths)]

Actually, to "avoid ABI breaks" we should be using the same compiler as all upstream and downstream projects in the archive. So this is wrong in principle.

review: Disapprove

[Colin Watson (cjwatson)]

We've talked about this extensively between Thomas and the Foundations
team (which owns the toolchain). Most of the archive is fine, including
most C++ projects, since libstdc++ in general offers a stable ABI except
when explicitly stated upstream; when we receive such statements from
upstream we deal with that differently, but that's not a problem at the
moment.

The specific problem at issue is that C++11 is experimental at the
toolchain level, and it isn't guaranteed that g++/libstdc++ will
preserve ABI stability there. We therefore need to make sure the
(small) set of packages using it are migrated together; this does *not*
need to be done in sync with the change of our default gcc/g++ version,
and in fact it's much easier to coordinate if it's decoupled. This is
why Thomas is working on all our C++11 packages to make all of them use
a specific version of g++, so that they can deal with orderly
transitions to new g++ versions on their own schedule with SONAME
changes etc. as required. This MP is one of many such.

For more details, please see:

  https://wiki.ubuntu.com/cpp-11

[Alan Griffiths (alan-griffiths)]

1. That would have been useful information in the "Description of the Change"

2. I can imagine that there are no attractive solutions: I don't immediately see a way to co-ordinate the projects that require the "many such" MPs.

3. I still don't like *this* solution as to "avoid ABI breaks" we should be using the same compiler as all upstream and downstream C++11 projects in the archive. So this is wrong in principle.

If we proceed with this solution, who is going to co-ordinate changes with other C++11 projects? (And how?)

review: Needs Information

[Daniel van Vugt (vanvugt)]

(1) debian/changelog modifications are way out of date. We're actually working on release 0.4.0 right now. The 0.2 series has its own branch. Regardless, all changes should start of the development branch so I think just drop the changelog modifications.

(2) Depending on g++-4.9 appears to be the first Mir code change to break compatibility with trusty. I guess that's OK so long as it's development branch and/or utopic branches only. Just worth mentioning.

(3) I'm a little concerned this could make people think _only_ gcc is supported. That's obviously not true as we can build with clang too.

The only real blocking issue is the changelog (1). It's also causing a merge conflict anyway.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

P.S. My preference is to Disapprove this. I don't think it's a good idea to enforce a specific compiler unless you have stronger reasons like the project only works with one specific compiler.

[Matthias Klose (doko)]

the phone team did choose to do this transition explicitly setting the compiler to g++-4.9. So there is no other way, and they already did start.

> (3) I'm a little concerned this could make people think
> _only_ gcc is supported. That's obviously not true
> as we can build with clang too.

no, clang is affected too, at least the version built in Ubuntu and configured to use libstdc++.

review: Approve

[Alan Griffiths (alan-griffiths)]

I'm still trying to understand what I'm missing and how this gains anything.

Without this change we'll experience "issues" as soon as any of our upstream, downstream or mir projects are built with an ABI incompatible compiler version. We then need to ensure that all relevant projects get rebuilt.

With this change we'll experience "issues" as soon as any of our upstream, downstream or mir projects are built with an ABI incompatible compiler version. We then need to ensure that all relevant projects get rebuilt.

The differences are:

1. In the first case the trigger is a new compiler in archive, in the second the trigger is some project updating an explicit dependency.

2. In the second case the explicit dependencies for all the relevant projects need to be updated in addition to forcing the rebuilds.

[Colin Watson (cjwatson)]

In the general case, until C++11 is stable at the toolchain level, it is possible that switching to a new compiler version may cause changes to exported symbols, requiring a SONAME change. It may therefore require source changes to projects using C++11 as well as simple rebuilds.

We don't want to have to coordinate changes to the default compiler version with things like SONAME changes in phone-related projects; it's better for those projects to be able to test the effects of this on their own schedule and do smaller coordinated transitions as required.

It remains necessary either way to coordinate changes to our C++11 projects, but at least this way it's trivial to grep the archive for the set of packages we need to worry about, and it's much easier to make sure that the compiler version change goes with a SONAME change if need be.

[Alan Griffiths (alan-griffiths)]

I don't think it's a good idea to enforce a specific compiler on a project and I remain to be convinced that this makes things any easier to coordinate for our C++11 projects.

But the debian/control is about how Canonical lands thing it its archive and the relevant consensus goes beyond the Mir project. If all the "phone-related projects" do this then we should go along as it doesn't make things much worse.

/me empathizes with David Cameron

review: Abstain

[Alexandros Frantzis (afrantzis)]

> But the debian/control is about how Canonical lands thing it its archive and the relevant consensus > goes beyond the Mir project. If all the "phone-related projects" do this then we should go along as > it doesn't make things much worse.

+1

review: Abstain

[Cemil Azizoglu (cemil-azizoglu)]

To me, coming from the mobile world, it's natural to stick with a particular, proven toolchain and assume others won't work until someone validates them and proves that they do. Toolchain bugs are usually difficult and labor intensive to spot and I'd rather not burn my cycles chasing them.

Since C++11 is experimental, it makes sense to want to have control over its use. Though I'd have to agree with others that this is not completely solving the problem. But with this, we can exercise a bit better control.

Also, having specific toolchain dependencies could prevent potential headaches for phone vendors using our software.

Need fixing :

As pointed out, drop the changelog mods.

review: Approve

[Matthias Klose (doko)]

Am 02.07.2014 15:17, schrieb Cemil Azizoglu:
> Also, having specific toolchain dependencies could prevent potential headaches for phone vendors using our software.

the problem here is the interaction with the other parts of the distribution.
you are not supposed to merge c++11 code build with different gcc-4.x versions
(until the c++11 support is called stable). So even if you default to 4.9, and
start building other distribution packages with 4.10 when the default gcc
changes to 4.10, you run into the same problem.

[Alan Griffiths (alan-griffiths)]

> To me, coming from the mobile world, it's natural to stick with a particular,
> proven toolchain and assume others won't work until someone validates them and
> proves that they do. Toolchain bugs are usually difficult and labor intensive
> to spot and I'd rather not burn my cycles chasing them.

I agree with all that.

> Since C++11 is experimental, it makes sense to want to have control over its
> use. Though I'd have to agree with others that this is not completely solving
> the problem. But with this, we can exercise a bit better control.

My disagreement is that introducing independent control for *each C++11* project isn't obviously any improvement. But if I don't have to use this mechanism co-ordinate the changes it isn't my concern. [Dons "someone else's problem" sunglasses]

> Also, having specific toolchain dependencies could prevent potential headaches
> for phone vendors using our software.

This doesn't dictate to phone vendors. The debian/control dependencies are simply what is used for the Canonical archive.

We also build and test with clang (but we don't publish that version). We can add other compilers.

> Need fixing :
>
> As pointed out, drop the changelog mods.

Yes, that's the only blocker. Thomas agreed to change that yesterday.

[Thomas Voß (thomas-voss)]

> > To me, coming from the mobile world, it's natural to stick with a
> particular,
> > proven toolchain and assume others won't work until someone validates them
> and
> > proves that they do. Toolchain bugs are usually difficult and labor
> intensive
> > to spot and I'd rather not burn my cycles chasing them.
>
> I agree with all that.
>
> > Since C++11 is experimental, it makes sense to want to have control over its
> > use. Though I'd have to agree with others that this is not completely
> solving
> > the problem. But with this, we can exercise a bit better control.
>
> My disagreement is that introducing independent control for *each C++11*
> project isn't obviously any improvement. But if I don't have to use this
> mechanism co-ordinate the changes it isn't my concern. [Dons "someone else's
> problem" sunglasses]
>
> > Also, having specific toolchain dependencies could prevent potential
> headaches
> > for phone vendors using our software.
>
> This doesn't dictate to phone vendors. The debian/control dependencies are
> simply what is used for the Canonical archive.
>
> We also build and test with clang (but we don't publish that version). We can
> add other compilers.
>
> > Need fixing :
> >
> > As pointed out, drop the changelog mods.
>
> Yes, that's the only blocker. Thomas agreed to change that yesterday.

Fixed.

[Cemil Azizoglu (cemil-azizoglu)]

> > Also, having specific toolchain dependencies could prevent potential
> headaches
> > for phone vendors using our software.
>
> This doesn't dictate to phone vendors. The debian/control dependencies are
> simply what is used for the Canonical archive.
>

Yes I realize that, but it'll offer a potential smoking gun for vendors to look at, when/if they see strange issues.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1729
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2059/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/798
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/804
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/797
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/580/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/578
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/578/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2441
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2441/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1949
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9171

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2059/rebuild

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

OK, since the only changes are under debian/ they can't affect or limit other distros or manual builds in any way. I don't like being distro-specific like this, but it's not a hard limitation either. I think it's rare you can port a project between distros without some packaging changes required.

review: Approve

[Alan Griffiths (alan-griffiths)]

"/tmp/buildd/mir-0.4.0bzr1730pkg0utopic580/src/shared/graphics/android/syncfence.cpp: In member function 'virtual void mir::graphics::android::SyncFence::merge_with(mir::graphics::android::NativeFence&)':
/tmp/buildd/mir-0.4.0bzr1730pkg0utopic580/src/shared/graphics/android/syncfence.cpp:68:51: error: overflow in implicit constant conversion [-Werror=overflow]
         ops->ioctl(fence_fd, SYNC_IOC_MERGE, &data);
                                                   ^
cc1plus: all warnings being treated as errors"

Is this what happens when one forces g++4.9?

In which case the code needs fixing first. (Probably clearer as a separate, prerequisite MP)

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1729
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2066/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/807
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/813
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/806
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/587/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/585
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/585/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2524
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2524/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1957
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9257

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2066/rebuild

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

Hmm, try building again? I just updated my utopic and forced an Android x-compile to use g++-4.9 and can't reproduce any error. I also can't see in code what the "implicit constant conversion" is.

[Alan Griffiths (alan-griffiths)]

> Hmm, try building again? I just updated my utopic and forced an Android
> x-compile to use g++-4.9 and can't reproduce any error. I also can't see in
> code what the "implicit constant conversion" is.

I can't reproduce either - but I already tried a rebuild.

[Alexandros Frantzis (afrantzis)]

I can reproduce, working on a fix.

=== modified file 'debian/control'
--- debian/control	2014-06-27 06:10:24 +0000
+++ debian/control	2014-07-02 13:45:40 +0000
@@ -10,6 +10,10 @@
                doxygen,
                xsltproc,
                graphviz,
+# We rely on C++11 features, and to prevent from ABI breaks
+# in libstdc++ causing us issues, we explicitly select a G++
+# version.
+               g++-4.9,
                libboost-dev,
                libboost-chrono-dev,
                libboost-date-time-dev,

=== modified file 'debian/rules'
--- debian/rules	2014-04-15 17:11:14 +0000
+++ debian/rules	2014-07-02 13:45:40 +0000
@@ -6,6 +6,13 @@
 
 export DPKG_GENSYMBOLS_CHECK_LEVEL = 4
 
+include /usr/share/dpkg/default.mk
+
+# Explicitly selecting a G{CC,++}-version here to avoid accidental
+# ABI breaks introduced by toolchain updates.
+export CC=$(DEB_HOST_GNU_TYPE)-gcc-4.9
+export CXX=$(DEB_HOST_GNU_TYPE)-g++-4.9
+
 %:
 	dh $@ --parallel --fail-missing