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

[Michael Vogt]

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

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

[Michael Vogt (mvo)]

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

[PS Jenkins bot (ps-jenkins)]

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)

[Colin Watson (cjwatson)]

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

[Michael Vogt (mvo)]

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 on 2014-05-07

remove get_pkgdir_from_appid() helper which was misguided

147. By Michael Vogt on 2014-05-07

add tests, make code more consitent

146. By Michael Vogt on 2014-05-07

fix error handling in get_manifest_for_app_id()

145. By Michael Vogt on 2014-05-06

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

=== modified file 'desktop-hook.c'
--- desktop-hook.c	2014-03-07 12:33:01 +0000
+++ desktop-hook.c	2014-05-07 08:02:18 +0000
@@ -105,6 +105,40 @@
 	return time;
 }
 
+/* Check if the required frameworks are available for the given appid */
+static gboolean
+has_required_frameworks(app_state_t * state)
+{
+        JsonObject * manifest = NULL;
+        gchar *framework = NULL;
+        gboolean res = FALSE;
+
+        manifest = get_manifest_for_appid(state->app_id);
+        if (manifest == NULL) {
+               g_warning("Manifest '%s' can not be read", state->app_id);
+               goto out;
+        }
+        if (!json_object_has_member(manifest, "framework")) {
+		g_warning("Manifest '%s' doesn't have an framework section", state->app_id);
+		goto out;
+	}
+        framework = g_strdup(json_object_get_string_member(manifest, "framework"));
+        if(framework == NULL) {
+                g_warning("Manifest '%s' doesn't have a framework", state->app_id);
+		goto out;
+        }
+        // FIXME: support multiple frameworks
+        res = click_framework_has_framework(framework);
+
+ out:
+	if (manifest != NULL)
+		json_object_unref(manifest);
+        g_free(framework);
+        return res;
+}
+
+
+
 /* Look at an click package entry */
 void
 add_click_package (const gchar * dir, const gchar * name, GArray * app_array)
@@ -120,6 +154,10 @@
 	state->has_click = TRUE;
 	state->click_modified = modified_time(dir, name);
 
+        // unregister if the required framework requirements are not meet
+        if (!has_required_frameworks(state))
+           state->has_click = FALSE;
+
 	g_free(appid);
 
 	return;

=== modified file 'helpers.c'
--- helpers.c	2014-03-12 00:28:05 +0000
+++ helpers.c	2014-05-07 08:02:18 +0000
@@ -57,10 +57,8 @@
 	return TRUE;
 }
 
-/* Take a manifest, parse it, find the application and
-   and then return the path to the desktop file */
-gchar *
-manifest_to_desktop (const gchar * app_dir, const gchar * app_id)
+JsonObject *
+get_manifest_for_appid(const gchar * app_id)
 {
 	gchar * package = NULL;
 	gchar * application = NULL;
@@ -68,7 +66,6 @@
 	ClickDB * db = NULL;
 	ClickUser * user = NULL;
 	JsonObject * manifest = NULL;
-	gchar * desktoppath = NULL;
 	GError * error = NULL;
 
 	if (!app_id_to_triplet(app_id, &package, &application, &version)) {
@@ -97,14 +94,51 @@
 
 	if (!json_object_has_member(manifest, "version")) {
 		g_warning("Manifest '%s' doesn't have a version", package);
+		json_object_unref(manifest);
+		manifest = NULL;
 		goto manifest_out;
 	}
 
 	if (g_strcmp0(json_object_get_string_member(manifest, "version"), version) != 0) {
 		g_warning("Manifest '%s' version '%s' doesn't match AppID version '%s'", package, json_object_get_string_member(manifest, "version"), version);
-		goto manifest_out;
-	}
-
+		json_object_unref(manifest);
+		manifest = NULL;
+		goto manifest_out;
+	}
+
+manifest_out:
+	if (error != NULL)
+		g_error_free(error);
+	g_clear_object(&user);
+	g_clear_object(&db);
+	g_free(package);
+	g_free(application);
+	g_free(version);
+
+        return manifest;
+}
+
+/* Take a manifest, parse it, find the application and
+   and then return the path to the desktop file */
+gchar *
+manifest_to_desktop (const gchar * app_dir, const gchar * app_id)
+{
+	gchar * desktoppath = NULL;
+	GError * error = NULL;
+	gchar * package = NULL;
+	gchar * application = NULL;
+	gchar * version = NULL;
+        JsonObject * manifest = NULL;
+
+	if (!app_id_to_triplet(app_id, &package, &application, &version)) {
+		g_warning("Unable to parse triplet: %s", app_id);
+		return NULL;
+	}
+
+        manifest = get_manifest_for_appid(app_id);
+        if (manifest == NULL) {
+		goto manifest_out;
+        }
 	if (!json_object_has_member(manifest, "hooks")) {
 		g_warning("Manifest '%s' doesn't have an hooks section", package);
 		goto manifest_out;
@@ -148,8 +182,6 @@
 		g_error_free(error);
 	if (manifest != NULL)
 		json_object_unref(manifest);
-	g_clear_object(&user);
-	g_clear_object(&db);
 	g_free(package);
 	g_free(application);
 	g_free(version);

=== modified file 'helpers.h'
--- helpers.h	2014-02-03 18:27:58 +0000
+++ helpers.h	2014-05-07 08:02:18 +0000
@@ -18,6 +18,7 @@
  */
 
 #include <glib.h>
+#include <json-glib/json-glib.h>
 
 gboolean  app_id_to_triplet      (const gchar *   app_id,
                                   gchar **        package,
@@ -37,7 +38,11 @@
 void      set_confined_envvars   (const gchar *   package,
                                   const gchar *   app_dir);
 
+JsonObject * get_manifest_for_appid (const gchar * app_id);
+
+
 typedef struct _handshake_t handshake_t;
 handshake_t * starting_handshake_start   (const gchar *   app_id);
 void      starting_handshake_wait        (handshake_t *   handshake);
 
+

=== modified file 'tests/helper-test.cc'
--- tests/helper-test.cc	2014-03-12 00:28:05 +0000
+++ tests/helper-test.cc	2014-05-07 08:02:18 +0000
@@ -453,3 +453,16 @@
 
 	return;
 }
+
+TEST_F(HelperTest, GetManifestForAppid)
+{
+	JsonObject *manifest = NULL;
+
+	g_setenv("TEST_CLICK_DB", "click-db-dir", TRUE);
+	g_setenv("TEST_CLICK_USER", "test-user", TRUE);
+
+	manifest = get_manifest_for_appid("com.test.good_application_1.2.3");
+	ASSERT_STREQ(json_object_get_string_member(manifest, "version"),
+                     "1.2.3");
+        json_object_unref(manifest);
+}