Merge lp://staging/~mvo/ubuntu-app-launch/hide-apps-on-missing-framework into lp://staging/ubuntu-app-launch/14.04

Proposed by Michael Vogt
Status: Work in progress
Proposed branch: lp://staging/~mvo/ubuntu-app-launch/hide-apps-on-missing-framework
Merge into: lp://staging/ubuntu-app-launch/14.04
Diff against target: 189 lines (+98/-10)
4 files modified
desktop-hook.c (+38/-0)
helpers.c (+42/-10)
helpers.h (+5/-0)
tests/helper-test.cc (+13/-0)
To merge this branch: bzr merge lp://staging/~mvo/ubuntu-app-launch/hide-apps-on-missing-framework
Reviewer Review Type Date Requested Status
Colin Watson (community) Needs Information
PS Jenkins bot (community) continuous-integration Needs Fixing
Indicator Applet Developers Pending
Review via email: mp+218564@code.staging.launchpad.net

Commit message

Hide apps if the framework requirements are not/no longer meet (LP: #1271944)

Description of the change

This branch fixes bug #1271944 (as suggested by Colin) - when the desktop hook is run at system and session startup it will check if the framework requirements are meet. If that is not the case, the click package is hidden from the available applications. Once the frameworks becomes available again the click is made available again.

Please let me know what you think, happy to change the code/improve tests etc. There is currently some code duplication around the handling of TEST_CLICK_{DB,USER}. If that is a concern I can move it into a helper function "ClickUser * click_get_user()" or something similar.

Thanks,
 Michael

To post a comment you must log in.
Revision history for this message
Michael Vogt (mvo) wrote :

Please also let me know if you prefer the branch as a single commit instead of the current 4 (for a cleaner history).

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:148
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/~mvo/upstart-app-launch/hide-apps-on-missing-framework/+merge/218564/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/upstart-app-launch-ci/262/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/upstart-app-launch-utopic-amd64-ci/1
    SUCCESS: http://jenkins.qa.ubuntu.com/job/upstart-app-launch-utopic-armhf-ci/1
    SUCCESS: http://jenkins.qa.ubuntu.com/job/upstart-app-launch-utopic-i386-ci/1

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/upstart-app-launch-ci/262/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Colin Watson (cjwatson) wrote :

This probably does handle the narrow issue at hand of removing the affected .desktop files, but I think it's a mistake to do this in upstart-app-launch. Other hooks may be affected, and we shouldn't have to have every hook handle this independently in order to get consistent results. I would much prefer us to handle this centrally instead.

Why not have click's hook code simply remove the symlinks associated with apps whose framework dependencies aren't satisfied and then re-run the Exec command for each affected hook? The hook's Exec command is supposed to bring the system into sync with the contents of the symlink farm populated by click, so removing entries from that symlink farm should be sufficient to make the .desktop files vanish when the Exec command is run, without the need to change upstart-app-launch at all.

review: Needs Information
Revision history for this message
Michael Vogt (mvo) wrote :

Thanks for your feedback Colin! I misunderstood the purpose of upstart-app-launch and it does indeed make more sense to have it in the central click hook code. I will rework the code accordingly.

Unmerged revisions

148. By Michael Vogt

remove get_pkgdir_from_appid() helper which was misguided

147. By Michael Vogt

add tests, make code more consitent

146. By Michael Vogt

fix error handling in get_manifest_for_app_id()

145. By Michael Vogt

unregister a click application if the required framework is not/no longer available

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