Merge lp://qastaging/~mterry/unity8/snap-root into lp://qastaging/unity8

Proposed by Michael Terry
Status: Merged
Approved by: Lukáš Tinkl
Approved revision: 2624
Merged at revision: 2661
Proposed branch: lp://qastaging/~mterry/unity8/snap-root
Merge into: lp://qastaging/unity8
Prerequisite: lp://qastaging/~unity-team/unity8/ual-launcher
Diff against target: 131 lines (+34/-25)
1 file modified
include/paths.h.in (+34/-25)
To merge this branch: bzr merge lp://qastaging/~mterry/unity8/snap-root
Reviewer Review Type Date Requested Status
Lukáš Tinkl (community) Approve
Unity8 CI Bot continuous-integration Approve
Review via email: mp+307207@code.qastaging.launchpad.net

Commit message

Adjust root paths if we're running inside a snap.

Description of the change

 * Are there any related MPs required for this MP to build/function as expected? Please list.
 No

 * Did you perform an exploratory manual test run of your code change and
any related functionality?
 Yes, during testing of u8 snap. You can also test this by faking it and doing a "make install" and setting $SNAP.

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
 NA

 * If you changed the UI, has there been a design review?
 NA

To post a comment you must log in.
Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

Just a few minor nitpicks, otherwise looks good

review: Needs Fixing
2623. By Michael Terry

Review nits

Revision history for this message
Michael Terry (mterry) wrote :

> qgetenv returns a QByteArray; therefore you should be doing sth like this
> (in the unlikely event that SNAP contains UTF-8 characters):
>
> QString snapRoot = QFile::decodeName(qgetenv("SNAP"));

And the unlikely event that we're on a non-UTF-8 filesystem, which has been always(?) been the default on Ubuntu.

But you're right. It is more correct to decode it. Done.

> installRoot.clear() or ... = QString() would be more efficient imo

I'm actually using the distinction between null and empty strings in this MP. null is "no prefix -- i.e. non-installed" and empty is "no prefix, but installed". So I want to set just the empty string, not set the string null.

> the additional QString() cast is redundant (also found in other places, just mentioning here)

Fixed, though unrelated to my MP. :)

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:2622
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2302/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3035
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/1683
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/1683
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/1683
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3063
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2920
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2920/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2920
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2920/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2920
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2920/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2920
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2920/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2920
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2920/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2920
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2920/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2920
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2920/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2920
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2920/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2920
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2920/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2302/rebuild

review: Approve (continuous-integration)
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:2623
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2304/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3038
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/1686
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/1686
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/1686
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3066
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2923
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2923/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2923
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2923/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2923
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2923/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2923
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2923/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2923
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2923/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2923
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2923/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2923
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2923/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2923
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2923/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2923
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2923/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2304/rebuild

review: Approve (continuous-integration)
Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

Looks good, let's see how it works together with the silo

review: Approve
Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

Take it back, this is fatal, unity8 fails to start on my laptop with:

file:///build/unity8-VZMeoQ/unity8-8.14+16.04.20161003.1/qml/OrientedShell.qml: File not found

review: Needs Fixing
2624. By Michael Terry

Guard check for cleanliness, though it shouldn't matter

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:2624
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2309/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3043
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/1691
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/1691
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/1691
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3071
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2928
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2928/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2928
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2928/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2928
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2928/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2928
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2928/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2928
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2928/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2928
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2928/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2928
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2928/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2928
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2928/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2928
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2928/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2309/rebuild

review: Approve (continuous-integration)
Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

All good now

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