Merge lp://staging/~ted/ubuntu-app-launch/snap-icon-unbreak into lp://staging/ubuntu-app-launch

Proposed by Ted Gould
Status: Rejected
Rejected by: Ted Gould
Proposed branch: lp://staging/~ted/ubuntu-app-launch/snap-icon-unbreak
Merge into: lp://staging/ubuntu-app-launch
Prerequisite: lp://staging/~ted/ubuntu-app-launch/install-root
Diff against target: 40 lines (+30/-0)
1 file modified
libubuntu-app-launch/application-impl-snap.cpp (+30/-0)
To merge this branch: bzr merge lp://staging/~ted/ubuntu-app-launch/snap-icon-unbreak
Reviewer Review Type Date Requested Status
dobey (community) Disapprove
unity-api-1-bot continuous-integration Needs Fixing
Michael Terry Approve
Review via email: mp+311224@code.staging.launchpad.net

Commit message

Handle some common mistakes in making snap desktop files

Description of the change

:-( Handle bad snaps because they're everywhere :-(

To post a comment you must log in.
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:271
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/129/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/1107
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1114
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/905
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/905/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/905
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/905/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/905
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/905/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/905
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/905/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/905
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/905/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/905
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/905/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/129/rebuild

review: Approve (continuous-integration)
Revision history for this message
Michael Terry (mterry) wrote :

Maybe use a trailing slash for the has_prefix checks? Ideally we wouldn't match Exec=/snap/XXX/currentlybroken, although obviously such Exec lines are already troubled.

Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:272
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/130/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/1125
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1132
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/923
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/923/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/923
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/923/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/923
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/923/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/923
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/923/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/923
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/923/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/923
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/923/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/130/rebuild

review: Approve (continuous-integration)
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:273
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/131/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/1147
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1154
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/942
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/942/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/942
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/942/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/942
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/942/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/942
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/942/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/942
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/942/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/942
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/942/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/131/rebuild

review: Approve (continuous-integration)
Revision history for this message
Michael Terry (mterry) wrote :

This looks good to me, and seems to work fine in the u8 snap. Thanks!

review: Approve
Revision history for this message
dobey (dobey) wrote :

Why does this depend on a series of incomplete and unapproved branches? Cannot this change be made independently?

review: Needs Information
274. By Ted Gould

Pull through updates

Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

FAILED: Continuous integration, rev:274
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/148/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/1326/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1333
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1110
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1110/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1110/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1110
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1110/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1110/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1110
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1110/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1110/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/148/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
dobey (dobey) wrote :

Now there are a lot of completely unrelated changes.

review: Needs Fixing
Revision history for this message
dobey (dobey) wrote :

I've proposed https://code.launchpad.net/~dobey/ubuntu-app-launch/snap-icon-unbreak/+merge/314461 instead, which takes the original 40 line change to strip the prefixes from Icon entries, and also added tests, as well as handling a couple more cases I've found that weren't handled.

Revision history for this message
dobey (dobey) :
review: Disapprove
275. By Ted Gould

Merge install-root

Unmerged revisions

275. By Ted Gould

Merge install-root

274. By Ted Gould

Pull through updates

273. By Ted Gould

Need a slash in both

272. By Ted Gould

Put slashes in to be extra safe

271. By Ted Gould

Unhack some common snap icon mistakes

270. By Ted Gould

Grab install root

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