Merge lp:click/devel into lp:click

[Colin Watson]

Click 0.4.33: Add scope APIs; fix GC; add "click chroot --name"; make "click destroy" more robust; stop apps when uninstalling them.

  [ Pete Woods ]
  * Add scope-facing APIs to chroot build (LP: #1370727).

  [ Colin Watson ]
  * Warn that "click install" without a registration may result in later
    garbage-collection.
  * Rearrange garbage-collection to remove versions of packages that have no
    user registrations and are not running, rather than using the artificial
    @gcinuse registration which never really worked properly.
  * Run garbage-collection immediately before running system hooks on system
    startup (LP: #1342858).
  * Add new -n/--name option to "click chroot", defaulting to "click"
    (LP: #1364327).

  [ Michael Vogt ]
  * Make click destroy more robust by unmounting any mounted filesystem
    inside the schroot first (LP: #1346723).
  * Stop apps if necessary when uninstalling them (LP: #1232130).
  * Add new "click framework {info,get-field}" subcommands.

[Colin Watson (cjwatson)]

review: Approve

=== modified file 'click/chroot.py'
--- click/chroot.py	2014-08-06 22:38:57 +0000
+++ click/chroot.py	2014-09-29 14:09:31 +0000
@@ -77,6 +77,11 @@
         ],
     "ubuntu-sdk-14.04": [
         "cmake",
+        "google-mock:TARGET",
+        "libboost1.54-dev:TARGET",
+        "libjsoncpp-dev:TARGET",
+        "libprocess-cpp-dev:TARGET",
+        "libproperties-cpp-dev:TARGET",
         "libqt5svg5-dev:TARGET",
         "libqt5webkit5-dev:TARGET",
         "libqt5xmlpatterns5-dev:TARGET",
@@ -99,7 +104,13 @@
         ],
     "ubuntu-sdk-14.10": [
         "cmake",
+        "google-mock:TARGET",
+        "libboost1.55-dev:TARGET",
         "libcontent-hub-dev:TARGET",
+        "libjsoncpp-dev:TARGET",
+        "libnet-cpp-dev:TARGET",
+        "libprocess-cpp-dev:TARGET",
+        "libproperties-cpp-dev:TARGET",
         "libqt5keychain0:TARGET",
         "libqt5sensors5-dev:TARGET",
         "libqt5svg5-dev:TARGET",
@@ -546,12 +557,26 @@
         return self.clean()
 
     def destroy(self):
-        if not self.exists():
-            raise ClickChrootDoesNotExistException(
-                "Chroot %s does not exist" % self.full_name)
-        os.remove(self.chroot_config)
-        mount = "%s/%s" % (self.chroots_dir, self.full_name)
-        shutil.rmtree(mount)
+        # remove config
+        if os.path.exists(self.chroot_config):
+            os.remove(self.chroot_config)
+        # find all schroot mount points, this is actually quite complicated
+        mount_dir = os.path.abspath(
+            os.path.join(self.chroots_dir, "..", "mount"))
+        needle = os.path.join(mount_dir, self.full_name)
+        all_mounts = []
+        with open("/proc/mounts") as f:
+            for line in f.readlines():
+                mp = line.split()[1]
+                if mp.startswith(needle):
+                    all_mounts.append(mp)
+        # reverse order is important in case of submounts
+        for mp in sorted(all_mounts, key=len, reverse=True):
+            subprocess.call(["umount", mp])
+        # now remove the rest
+        chroot_dir = "%s/%s" % (self.chroots_dir, self.full_name)
+        if os.path.exists(chroot_dir):
+            shutil.rmtree(chroot_dir)
         return 0
 
     def begin_session(self):

=== modified file 'click/commands/chroot.py'
--- click/commands/chroot.py	2014-07-01 07:12:49 +0000
+++ click/commands/chroot.py	2014-09-29 14:09:31 +0000
@@ -61,7 +61,8 @@
             "debootstrap not installed and configured; install click-dev and "
             "debootstrap")
     requires_root(parser)
-    chroot = ClickChroot(args.architecture, args.framework, series=args.series)
+    chroot = ClickChroot(
+        args.architecture, args.framework, name=args.name, series=args.series)
     with message_on_error(
             ClickChrootAlreadyExistsException, ErrorMessages.EXISTS):
         return chroot.create(args.keep_broken_chroot)
@@ -71,7 +72,7 @@
 
 def install(parser, args):
     packages = args.packages
-    chroot = ClickChroot(args.architecture, args.framework)
+    chroot = ClickChroot(args.architecture, args.framework, name=args.name)
     with message_on_error(
             ClickChrootDoesNotExistException, ErrorMessages.NOT_EXISTS):
         return chroot.install(*packages)
@@ -82,7 +83,7 @@
 def destroy(parser, args):
     requires_root(parser)
     # ask for confirmation?
-    chroot = ClickChroot(args.architecture, args.framework)
+    chroot = ClickChroot(args.architecture, args.framework, name=args.name)
     with message_on_error(
             ClickChrootDoesNotExistException, ErrorMessages.NOT_EXISTS):
         return chroot.destroy()
@@ -95,7 +96,8 @@
     if not program:
         program = ["/bin/bash"]
     chroot = ClickChroot(
-        args.architecture, args.framework, session=args.session)
+        args.architecture, args.framework, name=args.name,
+        session=args.session)
     with message_on_error(
             ClickChrootDoesNotExistException, ErrorMessages.NOT_EXISTS):
         return chroot.run(*program)
@@ -108,7 +110,8 @@
     if not program:
         program = ["/bin/bash"]
     chroot = ClickChroot(
-        args.architecture, args.framework, session=args.session)
+        args.architecture, args.framework, name=args.name,
+        session=args.session)
     with message_on_error(
             ClickChrootDoesNotExistException, ErrorMessages.NOT_EXISTS):
         return chroot.maint(*program)
@@ -117,7 +120,7 @@
 
 
 def upgrade(parser, args):
-    chroot = ClickChroot(args.architecture, args.framework)
+    chroot = ClickChroot(args.architecture, args.framework, name=args.name)
     with message_on_error(
             ClickChrootDoesNotExistException, ErrorMessages.NOT_EXISTS):
         return chroot.upgrade()
@@ -127,7 +130,8 @@
 
 def begin_session(parser, args):
     chroot = ClickChroot(
-        args.architecture, args.framework, session=args.session)
+        args.architecture, args.framework, name=args.name,
+        session=args.session)
     with message_on_error(
             ClickChrootDoesNotExistException, ErrorMessages.NOT_EXISTS):
         return chroot.begin_session()
@@ -137,7 +141,8 @@
 
 def end_session(parser, args):
     chroot = ClickChroot(
-        args.architecture, args.framework, session=args.session)
+        args.architecture, args.framework, name=args.name,
+        session=args.session)
     with message_on_error(
             ClickChrootDoesNotExistException, ErrorMessages.NOT_EXISTS):
         return chroot.end_session()
@@ -146,7 +151,7 @@
 
 
 def exists(parser, args):
-    chroot = ClickChroot(args.architecture, args.framework)
+    chroot = ClickChroot(args.architecture, args.framework, name=args.name)
     # return shell exit codes 0 on success, 1 on failure
     if chroot.exists():
         return 0
@@ -169,6 +174,11 @@
         "-s", "--series",
         help="series to use for a newly-created chroot (defaults to a series "
              "appropriate for the framework)")
+    parser.add_argument(
+        "-n", "--name", default="click",
+        help=(
+            "name of the chroot (default: click; the framework and "
+            "architecture will be appended)"))
     create_parser = subparsers.add_parser(
         "create",
         help="create a chroot of the provided architecture")

=== modified file 'click/commands/framework.py'
--- click/commands/framework.py	2014-05-20 07:02:53 +0000
+++ click/commands/framework.py	2014-09-29 14:09:31 +0000
@@ -28,6 +28,17 @@
     return 0
 
 
+def info(parser, args):
+    framework = Click.Framework.open(args.framework_name)
+    for field in sorted(framework.get_fields()):
+        print("%s: %s" % (field, framework.get_field(field)))
+
+
+def get_field(parser, args):
+    framework = Click.Framework.open(args.framework_name)
+    print(framework.get_field(args.field_name))
+
+
 def run(argv):
     parser = ArgumentParser("click framework")
     subparsers = parser.add_subparsers()
@@ -35,6 +46,24 @@
         "list",
         help="list available frameworks")
     list_parser.set_defaults(func=list)
+    info_parser = subparsers.add_parser(
+        "info",
+        help="show info about a specific framework")
+    info_parser.add_argument(
+        "framework_name",
+        help="framework name with the information")
+    info_parser.set_defaults(func=info)
+    get_field_parser = subparsers.add_parser(
+        "get-field",
+        help="get a field from a given framework")
+    get_field_parser.add_argument(
+        "framework_name",
+        help="framework name with the information")
+    get_field_parser.add_argument(
+        "field_name",
+        help="the field name (e.g. base-version)")
+    get_field_parser.set_defaults(func=get_field)
+
     args = parser.parse_args(argv)
     if not hasattr(args, "func"):
         parser.print_help()

=== modified file 'click/commands/install.py'
--- click/commands/install.py	2014-08-07 21:25:48 +0000
+++ click/commands/install.py	2014-09-29 14:09:31 +0000
@@ -45,7 +45,7 @@
         help="register package for all users")
     parser.add_option(
         "--allow-unauthenticated", default=False, action="store_true",
-        help="allow installing packages with no sigantures")
+        help="allow installing packages with no signatures")
     options, args = parser.parse_args(argv)
     if len(args) < 1:
         parser.error("need package file name")

=== modified file 'click/install.py'
--- click/install.py	2014-09-04 14:15:08 +0000
+++ click/install.py	2014-09-29 14:09:31 +0000
@@ -38,6 +38,7 @@
 import subprocess
 import sys
 import tempfile
+from textwrap import dedent
 
 from contextlib import closing
 
@@ -450,6 +451,12 @@
             else:
                 registry = Click.User.for_user(self.db, name=user)
             registry.set_version(package_name, package_version)
+        else:
+            print(dedent("""\
+                %s %s has not been registered for any users.
+                It may be garbage-collected the next time the system starts.
+                To avoid this, use "click register".
+                """) % (package_name, package_version))
 
         if old_version is not None:
             self.db.maybe_remove(package_name, old_version)

=== modified file 'click/tests/helpers.py'
--- click/tests/helpers.py	2014-06-23 21:14:06 +0000
+++ click/tests/helpers.py	2014-09-29 14:09:31 +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, mode)

=== modified file 'click/tests/integration/test_chroot.py'
--- click/tests/integration/test_chroot.py	2014-09-05 09:04:33 +0000
+++ click/tests/integration/test_chroot.py	2014-09-29 14:09:31 +0000
@@ -27,52 +27,58 @@
 class TestChroot(ClickTestCase):
 
     @classmethod
+    def command(cls, arch, *args):
+        return [cls.click_binary, "chroot", "-a", arch] + list(args)
+
+    @classmethod
     def setUpClass(cls):
         super(TestChroot, cls).setUpClass()
         require_root()
         require_network()
         cls.arch = subprocess.check_output(
             ["dpkg", "--print-architecture"], universal_newlines=True).strip()
-        subprocess.check_call([
-            cls.click_binary,
-            "chroot", "-a", cls.arch,
-            "create"])
+        subprocess.check_call(cls.command(cls.arch, "create"))
 
     @classmethod
     def tearDownClass(cls):
-        subprocess.check_call([
-            cls.click_binary,
-            "chroot", "-a", cls.arch,
-            "destroy"])
+        subprocess.check_call(cls.command(cls.arch, "destroy"))
 
     def test_upgrade(self):
-        subprocess.check_call([
-            self.click_binary, "chroot", "-a", self.arch,
-            "upgrade"])
+        subprocess.check_call(self.command(self.arch, "upgrade"))
 
     def test_install(self):
-        subprocess.check_call([
-            self.click_binary, "chroot", "-a", self.arch,
-            "install", "apt-utils"])
+        subprocess.check_call(self.command(self.arch, "install", "apt-utils"))
 
     def test_run(self):
-        output = subprocess.check_output([
-            self.click_binary, "chroot", "-a", self.arch,
-            "run", "echo", "hello world"], universal_newlines=True)
+        output = subprocess.check_output(
+            self.command(self.arch, "run", "echo", "hello world"),
+            universal_newlines=True)
         self.assertEqual(output, "hello world\n")
 
     def test_maint(self):
-        output = subprocess.check_output([
-            self.click_binary, "chroot", "-a", self.arch,
-            "maint", "id"], universal_newlines=True)
+        output = subprocess.check_output(
+            self.command(self.arch, "maint", "id"),
+            universal_newlines=True)
         self.assertEqual(output, "uid=0(root) gid=0(root) groups=0(root)\n")
 
     def test_exists_ok(self):
-        subprocess.check_call([
-            self.click_binary, "chroot", "-a", self.arch, "exists"])
+        subprocess.check_call(self.command(self.arch, "exists"))
 
     def test_exists_no(self):
         with self.assertRaises(subprocess.CalledProcessError):
-            subprocess.check_call([
-                self.click_binary,
-                "chroot", "-a", "arch-that-does-not-exist"])
+            subprocess.check_call(self.command("arch-that-does-not-exist"))
+
+
+class TestChrootName(TestChroot):
+    """Run the chroot tests again with a different --name."""
+
+    @classmethod
+    def command(cls, arch, *args):
+        return super(TestChrootName, cls).command(
+            arch, "-n", "testname", *args)
+
+    def test_exists_different_name_fails(self):
+        # "click chroot exists" fails for a non-existent name.
+        with self.assertRaises(subprocess.CalledProcessError):
+            subprocess.check_call(super(TestChrootName, self).command(
+                self.arch, "-n", "testname2", "exists"))

=== modified file 'click/tests/test_database.py'
--- click/tests/test_database.py	2014-07-11 17:20:51 +0000
+++ click/tests/test_database.py	2014-09-29 14:09:31 +0000
@@ -331,14 +331,6 @@
                 self.g_spawn_sync_side_effect, {b"ubuntu-app-pid": 0})
             preloads["click_find_on_path"].return_value = True
             self.db.maybe_remove("a", "1.0")
-            gcinuse_path = os.path.join(
-                self.temp_dir, ".click", "users", "@gcinuse", "a")
-            self.assertTrue(os.path.islink(gcinuse_path))
-            self.assertEqual(version_path, os.readlink(gcinuse_path))
-            self.assertTrue(os.path.exists(version_path))
-            self.db.maybe_remove("a", "1.0")
-            self.assertTrue(os.path.islink(gcinuse_path))
-            self.assertEqual(version_path, os.readlink(gcinuse_path))
             self.assertTrue(os.path.exists(version_path))
 
     def test_maybe_remove_not_running(self):
@@ -358,9 +350,6 @@
                 self.g_spawn_sync_side_effect, {b"ubuntu-app-pid": 1 << 8})
             preloads["click_find_on_path"].return_value = True
             self.db.maybe_remove("a", "1.0")
-            gcinuse_path = os.path.join(
-                self.temp_dir, ".click", "users", "@gcinuse", "a")
-            self.assertFalse(os.path.islink(gcinuse_path))
             self.assertFalse(os.path.exists(os.path.join(self.temp_dir, "a")))
 
     def test_gc(self):
@@ -399,8 +388,9 @@
             preloads["click_find_on_path"].return_value = True
             self.db.gc()
             self.assertTrue(os.path.exists(a_path))
+            self.assertFalse(os.path.exists(b_gcinuse_path))
             self.assertFalse(os.path.exists(b_path))
-            self.assertTrue(os.path.exists(c_path))
+            self.assertFalse(os.path.exists(c_path))
 
     def test_gc_ignores_non_directory(self):
         with self.run_in_subprocess(

=== 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 14:09:31 +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 'debian/changelog'
--- debian/changelog	2014-09-09 10:02:01 +0000
+++ debian/changelog	2014-09-29 14:09:31 +0000
@@ -1,3 +1,27 @@
+click (0.4.33) UNRELEASED; urgency=medium
+
+  [ Pete Woods ]
+  * Add scope-facing APIs to chroot build (LP: #1370727).
+
+  [ Colin Watson ]
+  * Warn that "click install" without a registration may result in later
+    garbage-collection.
+  * Rearrange garbage-collection to remove versions of packages that have no
+    user registrations and are not running, rather than using the artificial
+    @gcinuse registration which never really worked properly.
+  * Run garbage-collection immediately before running system hooks on system
+    startup (LP: #1342858).
+  * Add new -n/--name option to "click chroot", defaulting to "click"
+    (LP: #1364327).
+
+  [ Michael Vogt ]
+  * Make click destroy more robust by unmounting any mounted filesystem
+    inside the schroot first (LP: #1346723).
+  * Stop apps if necessary when uninstalling them (LP: #1232130).
+  * Add new "click framework {info,get-field}" subcommands.
+
+ -- Colin Watson <[e-mail]>  Wed, 10 Sep 2014 12:47:46 +0100
+
 click (0.4.32.1) utopic; urgency=low
 
   [ Michael Vogt ]

=== modified file 'doc/manpage.rst'
--- doc/manpage.rst	2014-09-05 16:18:38 +0000
+++ doc/manpage.rst	2014-09-29 14:09:31 +0000
@@ -245,6 +245,12 @@
 development versions of SDKs which have not yet put a framework declaration
 in place.
 
+You should always register installed packages either for a specific user or
+for all users; if you do not do this then the packages may be
+garbage-collected later.  You can do this using the ``--user`` or
+``--all-users`` options to this command, or using the ``click register``
+command.
+
 Options:
 
 --root=PATH                 Install packages underneath PATH.

=== modified file 'lib/click/database.vala'
--- lib/click/database.vala	2014-06-23 16:00:56 +0000
+++ lib/click/database.vala	2014-09-29 14:09:31 +0000
@@ -330,12 +330,8 @@
 	private void
 	remove_unless_running (string package, string version) throws Error
 	{
-		if (any_app_running (package, version)) {
-			var gc_in_use_user_db =
-				new User.for_gc_in_use (master_db);
-			gc_in_use_user_db.set_version (package, version);
+		if (any_app_running (package, version))
 			return;
-		}
 
 		var version_path = get_path (package, version);
 		if (show_messages ())
@@ -386,14 +382,10 @@
 	 * Remove a package version if it is not in use.
 	 *
 	 * "In use" may mean registered for another user, or running.  In
-	 * the latter case we construct a fake registration so that we can
-	 * tell the difference later between a package version that was in
-	 * use at the time of removal and one that was never registered for
-	 * any user.
-	 *
-	 * (This is unfortunately complex, and perhaps some day we can
-	 * require that installations always have some kind of registration
-	 * to avoid this complexity.)
+	 * either case, we do nothing.  We will already have removed at
+	 * least one registration by this point, so if no registrations are
+	 * left but it is running, then gc will be able to come along later
+	 * and clean things out.
 	 */
 	public void
 	maybe_remove (string package, string version) throws Error
@@ -408,11 +400,8 @@
 				continue;
 			}
 			if (reg_version == version) {
-				if (user_db.is_gc_in_use)
-					user_db.remove (package);
-				else
-					/* In use. */
-					return;
+				/* In use. */
+				return;
 			}
 		}
 
@@ -422,33 +411,33 @@
 	/**
 	 * gc:
 	 *
-	 * Remove package versions with no user registrations.
-	 *
-	 * To avoid accidentally removing packages that were installed
-	 * without ever having a user registration, we only garbage-collect
-	 * packages that were not removed by maybe_remove() due to having a
-	 * running application at the time.
-	 *
-	 * (This is unfortunately complex, and perhaps some day we can
-	 * require that installations always have some kind of registration
-	 * to avoid this complexity.)
+	 * Remove package versions that have no user registrations and that
+	 * are not running.
+	 *
+	 * This is rather like maybe_remove, but is suitable for bulk use,
+	 * since it only needs to scan the database once rather than once
+	 * per package.
+	 *
+	 * For historical reasons, we don't count @gcinuse as a real user
+	 * registration, and remove any such registrations we find.  We can
+	 * drop this once we no longer care about upgrading versions from
+	 * before this change to something more current in a single step.
 	 */
 	public void
 	gc () throws Error
 	{
 		var users_db = new Users (master_db);
 		var user_reg = new Gee.HashMultiMap<string, string> ();
-		var gc_in_use = new Gee.HashMultiMap<string, string> ();
 		foreach (var user_name in users_db.get_user_names ()) {
 			var user_db = users_db.get_user (user_name);
 			foreach (var package in user_db.get_package_names ()) {
 				var version = user_db.get_version (package);
+				if (version == "current")
+					continue;
 				/* Odd multimap syntax; this should really
 				 * be more like foo[package] += version.
 				 */
-				if (user_db.is_gc_in_use)
-					gc_in_use[package] = version;
-				else
+				if (! user_db.is_gc_in_use)
 					user_reg[package] = version;
 			}
 		}
@@ -462,22 +451,14 @@
 				continue;
 			foreach (var version in Click.Dir.open
 					(package_path)) {
+				if (version == "current")
+					continue;
 				if (version in user_reg[package])
 					/* In use. */
 					continue;
-				if (! (version in gc_in_use[package])) {
-					if (show_messages ()) {
-						var version_path =
-							Path.build_filename
-							(package_path,
-							 version);
-						message ("Not removing %s " +
-							 "(never registered).",
-							 version_path);
-					}
-					continue;
-				}
-				gc_in_use_user_db.remove (package);
+				if (gc_in_use_user_db.has_package_name
+						(package))
+					gc_in_use_user_db.remove (package);
 				remove_unless_running (package, version);
 			}
 		}

=== modified file 'lib/click/hooks.vala'
--- lib/click/hooks.vala	2014-05-19 13:19:16 +0000
+++ lib/click/hooks.vala	2014-09-29 14:09:31 +0000
@@ -1170,6 +1170,7 @@
 public void
 run_system_hooks (DB db) throws Error
 {
+	db.gc ();
 	db.ensure_ownership ();
 	string[] failed = {};
 	foreach (var hook in Hook.open_all (db)) {

=== modified file 'lib/click/user.vala'
--- lib/click/user.vala	2014-07-11 17:16:35 +0000
+++ lib/click/user.vala	2014-09-29 14:09:31 +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);
 	}