Merge lp:~mvo/click/lp1232130-kill-on-remove into lp:click/devel

[Michael Vogt]

Stop apps if necessary when uninstalling them.

Another branch where I would love to get some feedback if the direction looks good. If so, I will refactor the code to be cleaner. Please let me know if you would prefer to avoid the new make_file_with_content() helper and if you want me to use mkfile instead (maybe my version is a bit too specific for a general test helper).

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:430
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/click/lp1232130-kill-on-remove/+merge/236298/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/click-devel-ci/79/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/click-devel-utopic-amd64-ci/81
    SUCCESS: http://jenkins.qa.ubuntu.com/job/click-devel-utopic-armhf-ci/79
        deb: http://jenkins.qa.ubuntu.com/job/click-devel-utopic-armhf-ci/79/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/click-devel-utopic-i386-ci/79

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/click-devel-ci/79/rebuild

review: Needs Fixing (continuous-integration)

[Colin Watson (cjwatson)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:430
http://jenkins.qa.ubuntu.com/job/click-devel-ci/83/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/click-devel-utopic-amd64-ci/85
    SUCCESS: http://jenkins.qa.ubuntu.com/job/click-devel-utopic-armhf-ci/83
        deb: http://jenkins.qa.ubuntu.com/job/click-devel-utopic-armhf-ci/83/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/click-devel-utopic-i386-ci/83

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/click-devel-ci/83/rebuild

review: Approve (continuous-integration)

=== modified file 'click/tests/helpers.py'
--- click/tests/helpers.py	2014-06-23 21:14:06 +0000
+++ click/tests/helpers.py	2014-09-29 11:15:22 +0000
@@ -326,3 +326,11 @@
 def touch(path):
     with mkfile(path, mode="a"):
         pass
+
+
+def make_file_with_content(filename, content, mode=0o644):
+    """Create a file with the given content and mode"""
+    Click.ensuredir(os.path.dirname(filename))
+    with open(filename, "w") as f:
+        f.write(content)
+    os.chmod(filename, 0o755)

=== modified file 'click/tests/test_user.py'
--- click/tests/test_user.py	2014-07-09 12:56:21 +0000
+++ click/tests/test_user.py	2014-09-29 11:15:22 +0000
@@ -26,12 +26,18 @@
 import json
 import os
 import shutil
+from textwrap import dedent
 
 from gi.repository import Click, GLib
 
 from click.json_helpers import json_array_to_python, json_object_to_python
 from click.tests.gimock_types import Passwd
-from click.tests.helpers import TestCase, mkfile, touch
+from click.tests.helpers import (
+    TestCase,
+    mkfile,
+    make_file_with_content,
+    touch,
+)
 
 
 class TestClickUser(TestCase):
@@ -53,7 +59,8 @@
         a_1_0 = os.path.join(self.temp_dir, "custom", "a", "1.0")
         os.makedirs(a_1_0)
         with mkfile(os.path.join(a_1_0, ".click", "info", "a.manifest")) as m:
-            json.dump({"name": "a", "version": "1.0"}, m)
+            json.dump({"name": "a", "version": "1.0",
+                       "hooks": {"a-app": {}}}, m)
         b_2_0 = os.path.join(self.temp_dir, "custom", "b", "2.0")
         os.makedirs(b_2_0)
         with mkfile(os.path.join(b_2_0, ".click", "info", "b.manifest")) as m:
@@ -549,3 +556,22 @@
         self.assertEqual("2.0", registry.get_version("b"))
         self.assertEqual(b_overlay, registry.get_path("b"))
         self.assertTrue(registry.is_removable("b"))
+
+    def test_app_stops_on_remove(self):
+        fake_app_stop = os.path.join(self.temp_dir, "bin", "ubuntu-app-stop")
+        fake_app_stop_output = os.path.join(self.temp_dir, "fake-app-stop.out")
+        fake_app_stop_content = dedent("""\
+        #!/bin/sh
+        echo "$@" >> %s
+        """ % fake_app_stop_output)
+        make_file_with_content(fake_app_stop, fake_app_stop_content, 0o755)
+        # its ok to modify env here, click.helpers.TestCase will take care
+        # of it
+        os.environ["PATH"] = "%s:%s" % (
+            os.path.dirname(fake_app_stop), os.environ["PATH"])
+        # get a app with manifest etc all
+        _, registry = self._setUpMultiDB()
+        registry.remove("a")
+        # ensure that stop was called with the right app
+        with open(fake_app_stop_output) as f:
+            self.assertEqual("a_a-app_1.1", f.read().strip())

=== modified file 'lib/click/user.vala'
--- lib/click/user.vala	2014-07-11 17:16:35 +0000
+++ lib/click/user.vala	2014-09-29 11:15:22 +0000
@@ -598,6 +598,48 @@
 					       old_version, version, name);
 	}
 
+	private bool
+    stop_running_app (string package, string version)
+	{
+		var res = true;
+		if (! find_on_path ("ubuntu-app-stop"))
+			return false;
+
+		Json.Object manifest;
+		try {
+			manifest = get_manifest (package);
+		} catch (Error e) {
+			warning ("Can not get manifest for %s", package);
+			return false;
+		}
+		
+		if (! manifest.has_member ("hooks")) {
+			warning ("No hooks in manifest %s", package);
+			return false;
+		}
+		var hooks = manifest.get_object_member ("hooks");
+		foreach (unowned string app_name in
+				 hooks.get_members ())
+		{
+			// FIXME: move this into a "stop_single_app" helper
+			string[] command = {
+				"ubuntu-app-stop",
+				@"$(package)_$(app_name)_$(version)"
+			};
+			try {
+				int exit_status;
+				Process.spawn_sync (null, command, null,
+									SpawnFlags.SEARCH_PATH |
+									SpawnFlags.STDOUT_TO_DEV_NULL,
+									null, null, null, out exit_status);
+				res &= Process.check_exit_status (exit_status);
+			} catch (Error e) {
+				res &= false;
+			}
+		}
+		return res;
+	}
+
 	/**
 	 * remove:
 	 * @package: A package name.
@@ -636,6 +678,14 @@
 				regain_privileges ();
 			}
 		}
+
+		drop_privileges ();
+		try {
+			stop_running_app (package, old_version);
+		} finally {
+			regain_privileges ();
+		}
+
 		if (! is_pseudo_user)
 			package_remove_hooks (db, package, old_version, name);
 	}