Merge lp:click/devel into lp:click

[Michael Vogt]

Click 0.4.38: stop apps when uninstalling them, fix crash on empty db, add --ignore option to click build, fix framework validation for snappy frameworks

click (0.4.38) UNRELEASED; urgency=low

  * lp:~mvo/click/lp1232130-kill-on-remove-2:
    - When uninstalling a app, stop it if its running (LP: #1232130)
  * lp:~mvo/click/dont-crash-for-empty-db:
    - Do not crash when no database configuration is used and
      Click.DB.{get,props.overlay,gc,ensure_ownership} are called.
  * lp:~mvo/click/lp1219912-build-exclude:
    - add a new --ignore option to click {build,buildsource}
      (LP: #1219912)
  * lp:~mvo/click/fix-multiple-framework-validation:
    - fix framework validation for snappy

 -- Michael Vogt <email address hidden> Mon, 23 Feb 2015 08:20:04 +0100

[lp:click/devel]

563. By Michael Vogt on 2015-02-24

merged lp:~mvo/click/fix-options-ignore

564. By Michael Vogt on 2015-02-24

bump version number

565. By Michael Vogt on 2015-02-24

bump version number

566. By Michael Vogt on 2015-02-24

run debian/packagekit-check with "sh" as something in jenkins
made it mode 0644

567. By Michael Vogt on 2015-02-26

merged lp:~mvo/click/fix-autopkgtest

=== modified file 'click/build.py'
--- click/build.py	2015-01-19 10:26:26 +0000
+++ click/build.py	2015-02-26 17:06:20 +0000
@@ -83,6 +83,42 @@
 class ClickBuilderBase:
     def __init__(self):
         self.file_map = {}
+        # From @Dpkg::Source::Package::tar_ignore_default_pattern.
+        # (more in ClickSourceBuilder)
+        self._ignore_patterns = [
+            "*.click",
+            ".*.sw?",
+            "*~",
+            ",,*",
+            ".[#~]*",
+            ".arch-ids",
+            ".arch-inventory",
+            ".bzr",
+            ".bzr-builddeb",
+            ".bzr.backup",
+            ".bzr.tags",
+            ".bzrignore",
+            ".cvsignore",
+            ".git",
+            ".gitattributes",
+            ".gitignore",
+            ".gitmodules",
+            ".hg",
+            ".hgignore",
+            ".hgsigs",
+            ".hgtags",
+            ".shelf",
+            ".svn",
+            "CVS",
+            "DEADJOE",
+            "RCS",
+            "_MTN",
+            "_darcs",
+            "{arch}",
+        ]
+
+    def add_ignore_pattern(self, pattern):
+        self._ignore_patterns.append(pattern)
 
     def add_file(self, source_path, dest_path):
         self.file_map[source_path] = dest_path
@@ -132,39 +168,6 @@
 
 
 class ClickBuilder(ClickBuilderBase):
-    # TODO: This should be configurable, or at least extensible.
-    _ignore_patterns = [
-        "*.click",
-        ".*.sw?",
-        "*~",
-        ",,*",
-        ".[#~]*",
-        ".arch-ids",
-        ".arch-inventory",
-        ".be",
-        ".bzr",
-        ".bzr-builddeb",
-        ".bzr.backup",
-        ".bzr.tags",
-        ".bzrignore",
-        ".cvsignore",
-        ".git",
-        ".gitattributes",
-        ".gitignore",
-        ".gitmodules",
-        ".hg",
-        ".hgignore",
-        ".hgsigs",
-        ".hgtags",
-        ".shelf",
-        ".svn",
-        "CVS",
-        "DEADJOE",
-        "RCS",
-        "_MTN",
-        "_darcs",
-        "{arch}",
-        ]
 
     def list_files(self, root_path):
         for dirpath, _, filenames in os.walk(root_path):
@@ -298,44 +301,18 @@
 
 
 class ClickSourceBuilder(ClickBuilderBase):
-    # From @Dpkg::Source::Package::tar_ignore_default_pattern.
-    # TODO: This should be configurable, or at least extensible.
-    _ignore_patterns = [
-        "*.a",
-        "*.click",
-        "*.la",
-        "*.o",
-        "*.so",
-        ".*.sw?",
-        "*~",
-        ",,*",
-        ".[#~]*",
-        ".arch-ids",
-        ".arch-inventory",
-        ".be",
-        ".bzr",
-        ".bzr-builddeb",
-        ".bzr.backup",
-        ".bzr.tags",
-        ".bzrignore",
-        ".cvsignore",
-        ".deps",
-        ".git",
-        ".gitattributes",
-        ".gitignore",
-        ".gitmodules",
-        ".hg",
-        ".hgignore",
-        ".hgsigs",
-        ".hgtags",
-        ".shelf",
-        ".svn",
-        "CVS",
-        "DEADJOE",
-        "RCS",
-        "_MTN",
-        "_darcs",
-        "{arch}",
+
+    def __init__(self):
+        super(ClickSourceBuilder, self).__init__()
+        # From @Dpkg::Source::Package::tar_ignore_default_pattern.
+        # (more in ClickBuilderBase)
+        self._ignore_patterns += [
+            "*.a",
+            ".be",
+            ".deps",
+            "*.la",
+            "*.o",
+            "*.so",
         ]
 
     def build(self, dest_dir, manifest_path=None):

=== modified file 'click/commands/build.py'
--- click/commands/build.py	2014-09-05 14:03:19 +0000
+++ click/commands/build.py	2015-02-26 17:06:20 +0000
@@ -34,6 +34,9 @@
     parser.add_option(
         "--no-validate", action="store_false", default=True, dest="validate",
         help="Don't run click-reviewers-tools check on resulting .click")
+    parser.add_option(
+        "-I", "--ignore", metavar="file-pattern", action='append', default=[],
+        help="Ignore the given pattern when building the package")
     options, args = parser.parse_args(argv)
     if len(args) < 1:
         parser.error("need directory")
@@ -48,6 +51,8 @@
             (directory, options.manifest))
     builder = ClickBuilder()
     builder.add_file(directory, "./")
+    for ignore in options.ignore:
+        builder.add_ignore_pattern(ignore)
     try:
         path = builder.build(".", manifest_path=options.manifest)
     except ClickBuildError as e:

=== modified file 'click/commands/buildsource.py'
--- click/commands/buildsource.py	2014-06-17 08:25:27 +0000
+++ click/commands/buildsource.py	2015-02-26 17:06:20 +0000
@@ -29,6 +29,10 @@
     parser.add_option(
         "-m", "--manifest", metavar="PATH",
         help="read package manifest from PATH")
+    parser.add_option(
+        "-I", "--ignore", metavar="file-pattern", action='append',
+        default=[],
+        help="Ignore the given pattern when building the package")
     options, args = parser.parse_args(argv)
     if len(args) < 1:
         parser.error("need directory")
@@ -45,6 +49,8 @@
             (directory, options.manifest))
     builder = ClickSourceBuilder()
     builder.add_file(directory, "./")
+    for ignore in options.ignore:
+        builder.add_ignore_pattern(ignore)
     try:
         path = builder.build(".", manifest_path=options.manifest)
     except ClickBuildError as e:

=== modified file 'click/framework.py'
--- click/framework.py	2014-06-23 21:14:06 +0000
+++ click/framework.py	2015-02-26 17:06:20 +0000
@@ -31,12 +31,6 @@
     pass
 
 
-# FIXME: use native lib if available
-#from gi.repository import Click
-#click_framework_get_base_version = Click.framework_get_base_version
-#click_framework_has_framework = Click.has_framework
-
-
 # python version of the vala parse_deb822_file()
 def parse_deb822_file(filename):
     data = {}
@@ -76,6 +70,12 @@
     return deb822.get("base-version", None)
 
 
+# python version of the vala click_framework_get_base_version()
+def click_framework_get_base_name(framework_name):
+    deb822 = parse_deb822_file(get_framework_path(framework_name))
+    return deb822.get("base-name", None)
+
+
 # python version of the vala click_framework_has_framework
 def click_framework_has_framework(framework_name):
     return os.path.exists(get_framework_path(framework_name))
@@ -94,7 +94,7 @@
         raise ClickFrameworkInvalid(
             'Could not parse framework "%s"' % framework_string)
 
-    framework_base_versions = set()
+    base_name_versions = {}
     missing_frameworks = []
     for or_dep in parsed_framework:
         if len(or_dep) > 1:
@@ -110,9 +110,20 @@
         if not click_framework_has_framework(framework_name):
             missing_frameworks.append(framework_name)
             continue
+        # ensure we do not use different base versions for the same base-name
+        framework_base_name = click_framework_get_base_name(
+            framework_name)
         framework_base_version = click_framework_get_base_version(
             framework_name)
-        framework_base_versions.add(framework_base_version)
+        prev =  base_name_versions.get(framework_base_name, None)
+        if prev and prev != framework_base_version:
+            raise ClickFrameworkInvalid(
+                'Multiple frameworks with different base versions are not '
+                'allowed. Found: {} ({} != {})'.format(
+                    framework_base_name,
+                    framework_base_version,
+                    base_name_versions[framework_base_name]))
+        base_name_versions[framework_base_name] = framework_base_version
 
     if not ignore_missing_frameworks:
         if len(missing_frameworks) > 1:
@@ -132,8 +143,3 @@
         elif missing_frameworks:
             logging.warning('Ignoring missing framework "%s"' % (
                 missing_frameworks[0]))
-
-    if len(framework_base_versions) > 1:
-        raise ClickFrameworkInvalid(
-            'Multiple frameworks with different base versions are not '
-            'allowed. Found: {0}'.format(framework_base_versions))

=== modified file 'click/tests/helpers.py'
--- click/tests/helpers.py	2014-09-29 13:23:52 +0000
+++ click/tests/helpers.py	2015-02-26 17:06:20 +0000
@@ -30,6 +30,7 @@
 
 import contextlib
 from functools import wraps
+import json
 import os
 import re
 import shutil
@@ -59,6 +60,26 @@
     return wrapper
 
 
+def make_installed_click(db, db_dir, package="test-1", version="1.0",
+                         json_data={}, make_current=True, user="@all"):
+    """Create a fake installed click package for the given db/db_dir"""
+    json_data["name"] = package
+    json_data["version"] = version
+    with mkfile_utf8(os.path.join(
+            db_dir, package, version, ".click", "info",
+            "%s.manifest" % package)) as f:
+        json.dump(json_data, f, ensure_ascii=False)
+    if make_current:
+        os.symlink(
+            version, os.path.join(db_dir, package, "current"))
+    if user == "@all":
+        registry = Click.User.for_all_users(db)
+    else:
+        registry = Click.User.for_user(db, user)
+    registry.set_version(package, version)
+    return os.path.join(db_dir, package, version)
+
+
 class TestCase(gimock.GIMockTestCase):
     def setUp(self):
         super(TestCase, self).setUp()

=== modified file 'click/tests/test_build.py'
--- click/tests/test_build.py	2015-01-19 10:26:26 +0000
+++ click/tests/test_build.py	2015-02-26 17:06:20 +0000
@@ -207,21 +207,37 @@
         self.assertEqual(
             "foo", os.readlink(os.path.join(extract_path, "bin", "bar")))
 
+    def _make_scratch_dir(self, manifest_override={}):
+        self.use_temp_dir()
+        scratch = os.path.join(self.temp_dir, "scratch")
+        manifest = {
+                "name": "com.example.test",
+                "version": "1.0",
+                "maintainer": "Foo Bar <[e-mail]>",
+                "title": "test title",
+                "architecture": "all",
+                "framework": "ubuntu-sdk-13.10",
+        }
+        manifest.update(manifest_override)
+        with mkfile(os.path.join(scratch, "manifest.json")) as f:
+            json.dump(manifest, f)
+        self.builder.add_file(scratch, "/")
+        return scratch
+
     @disable_logging
     def test_build_excludes_dot_click(self):
-        self.use_temp_dir()
-        scratch = os.path.join(self.temp_dir, "scratch")
+        scratch = self._make_scratch_dir()
         touch(os.path.join(scratch, ".click", "evil-file"))
-        with mkfile(os.path.join(scratch, "manifest.json")) as f:
-            json.dump({
-                "name": "com.example.test",
-                "version": "1.0",
-                "maintainer": "Foo Bar <[e-mail]>",
-                "title": "test title",
-                "architecture": "all",
-                "framework": "ubuntu-sdk-13.10",
-            }, f)
+        path = self.builder.build(self.temp_dir)
+        extract_path = os.path.join(self.temp_dir, "extract")
+        subprocess.check_call(["dpkg-deb", "-x", path, extract_path])
+        self.assertEqual([], os.listdir(extract_path))
+
+    def test_build_ignore_pattern(self):
+        scratch = self._make_scratch_dir()
+        touch(os.path.join(scratch, "build", "foo.o"))
         self.builder.add_file(scratch, "/")
+        self.builder.add_ignore_pattern("build")
         path = self.builder.build(self.temp_dir)
         extract_path = os.path.join(self.temp_dir, "extract")
         subprocess.check_call(["dpkg-deb", "-x", path, extract_path])
@@ -229,18 +245,9 @@
 
     @disable_logging
     def test_build_multiple_architectures(self):
-        self.use_temp_dir()
-        scratch = os.path.join(self.temp_dir, "scratch")
-        with mkfile(os.path.join(scratch, "manifest.json")) as f:
-            json.dump({
-                "name": "com.example.test",
-                "version": "1.0",
-                "maintainer": "Foo Bar <[e-mail]>",
-                "title": "test title",
+        scratch = self._make_scratch_dir(manifest_override={
                 "architecture": ["armhf", "i386"],
-                "framework": "ubuntu-sdk-13.10",
-            }, f)
-        self.builder.add_file(scratch, "/")
+        })
         path = os.path.join(self.temp_dir, "com.example.test_1.0_multi.click")
         self.assertEqual(path, self.builder.build(self.temp_dir))
         self.assertTrue(os.path.exists(path))
@@ -255,22 +262,12 @@
             del target_json["installed-size"]
             self.assertEqual(source_json, target_json)
 
-    # FIXME: DRY violation with test_build_multiple_architectures etc
     @disable_logging
     def test_build_multiple_frameworks(self):
-        self.use_temp_dir()
-        scratch = os.path.join(self.temp_dir, "scratch")
-        with mkfile(os.path.join(scratch, "manifest.json")) as f:
-            json.dump({
-                "name": "com.example.test",
-                "version": "1.0",
-                "maintainer": "Foo Bar <[e-mail]>",
-                "title": "test title",
-                "architecture": "all",
+        scratch = self._make_scratch_dir(manifest_override={
                 "framework":
                     "ubuntu-sdk-14.04-basic, ubuntu-sdk-14.04-webapps",
-            }, f)
-        self.builder.add_file(scratch, "/")
+        })
         path = self.builder.build(self.temp_dir)
         control_path = os.path.join(self.temp_dir, "control")
         subprocess.check_call(["dpkg-deb", "-e", path, control_path])
@@ -289,13 +286,15 @@
         self.builder = ClickBuilder()
         for framework_name in ("ubuntu-sdk-13.10",
                                "ubuntu-sdk-14.04-papi",
-                               "ubuntu-sdk-14.04-html"):
+                               "ubuntu-sdk-14.04-html",
+                               "docker-sdk-1.3"):
             self._create_mock_framework_file(framework_name)
 
     def test_validate_framework_good(self):
         valid_framework_values = (
             "ubuntu-sdk-13.10",
             "ubuntu-sdk-14.04-papi, ubuntu-sdk-14.04-html",
+            "ubuntu-sdk-13.10, docker-sdk-1.3",
         )
         for framework in valid_framework_values:
             self.builder._validate_framework(framework)
@@ -322,6 +321,8 @@
         scratch = os.path.join(self.temp_dir, "scratch")
         touch(os.path.join(scratch, "bin", "foo"))
         touch(os.path.join(scratch, ".git", "config"))
+        touch(os.path.join(scratch, "foo.so"))
+        touch(os.path.join(scratch, "build", "meep.goah"))
         with mkfile(os.path.join(scratch, "manifest.json")) as f:
             json.dump({
                 "name": "com.example.test",
@@ -334,6 +335,7 @@
             # build() overrides this back to 0o644
             os.fchmod(f.fileno(), 0o600)
         self.builder.add_file(scratch, "./")
+        self.builder.add_ignore_pattern("build")
         path = os.path.join(self.temp_dir, "com.example.test_1.0.tar.gz")
         self.assertEqual(path, self.builder.build(self.temp_dir))
         self.assertTrue(os.path.exists(path))

=== modified file 'click/tests/test_database.py'
--- click/tests/test_database.py	2014-09-10 11:54:14 +0000
+++ click/tests/test_database.py	2015-02-26 17:06:20 +0000
@@ -562,6 +562,18 @@
         db = Click.DB()
         self.assertEqual(0, db.props.size)
 
+    def test_no_db_conf_errors(self):
+        db = Click.DB()
+        self.assertRaisesDatabaseError(
+            Click.DatabaseError.INVALID, db.get, 0)
+        self.assertEqual(db.props.overlay, "")
+        self.assertRaisesDatabaseError(
+            Click.DatabaseError.INVALID, db.maybe_remove, "something", "1.0")
+        self.assertRaisesDatabaseError(
+            Click.DatabaseError.INVALID, db.gc)
+        self.assertRaisesDatabaseError(
+            Click.DatabaseError.INVALID, db.ensure_ownership)
+
     def test_read_nonexistent(self):
         db = Click.DB()
         db.read(db_dir=os.path.join(self.temp_dir, "nonexistent"))

=== modified file 'click/tests/test_user.py'
--- click/tests/test_user.py	2014-09-29 11:12:52 +0000
+++ click/tests/test_user.py	2015-02-26 17:06:20 +0000
@@ -34,6 +34,7 @@
 from click.tests.gimock_types import Passwd
 from click.tests.helpers import (
     TestCase,
+    make_installed_click,
     mkfile,
     make_file_with_content,
     touch,
@@ -557,21 +558,34 @@
         self.assertEqual(b_overlay, registry.get_path("b"))
         self.assertTrue(registry.is_removable("b"))
 
-    def test_app_stops_on_remove(self):
+
+class StopAppTestCase(TestCase):
+
+    def setUp(self):
+        super(StopAppTestCase, self).setUp()
+        self.use_temp_dir()
+        self.db = Click.DB()
+        self.db.add(self.temp_dir)
+
+        # setup fake app_stop
         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")
+        self.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)
+        """ % self.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")
+
+    def test_app_stops_on_remove(self):
+        make_installed_click(self.db, self.temp_dir, "meep", "2.0",
+                             {"hooks": {"a-app": {}}})
+        registry = Click.User.for_user(self.db, "user")
+        registry.remove("meep")
         # 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())
+        with open(self.fake_app_stop_output) as f:
+            self.assertEqual("meep_a-app_2.0", f.read().strip())

=== modified file 'debian/changelog'
--- debian/changelog	2015-02-13 14:46:35 +0000
+++ debian/changelog	2015-02-26 17:06:20 +0000
@@ -1,3 +1,20 @@
+click (0.4.38.4) UNRELEASED; urgency=low
+
+  * lp:~mvo/click/lp1232130-kill-on-remove-2:
+    - When uninstalling a app, stop it if its running (LP: #1232130)
+  * lp:~mvo/click/dont-crash-for-empty-db:
+    - Do not crash when no database configuration is used and
+      Click.DB.{get,props.overlay,gc,ensure_ownership} are called.
+  * lp:~mvo/click/lp1219912-build-exclude:
+    - add a new --ignore option to click {build,buildsource}
+      (LP: #1219912)
+  * lp:~mvo/click/fix-multiple-framework-validation:
+    - fix framework validation for snappy
+  * run debian/packagekit-check with "sh" as something in jenkins
+    made it mode 0644
+
+ -- Michael Vogt <[e-mail]>  Mon, 23 Feb 2015 08:20:04 +0100
+
 click (0.4.37) vivid; urgency=low
 
   [ Michael Vogt ]

=== modified file 'debian/rules'
--- debian/rules	2014-10-10 07:10:23 +0000
+++ debian/rules	2015-02-26 17:06:20 +0000
@@ -1,6 +1,6 @@
 #! /usr/bin/make -f
 
-PACKAGEKIT := $(shell debian/packagekit-check)
+PACKAGEKIT := $(shell sh debian/packagekit-check)
 ifeq ($(PACKAGEKIT),yes)
 EXTRA_DH_OPTIONS :=
 else

=== modified file 'doc/manpage.rst'
--- doc/manpage.rst	2014-10-13 12:48:35 +0000
+++ doc/manpage.rst	2015-02-26 17:06:20 +0000
@@ -73,6 +73,11 @@
 
 -m PATH, --manifest=PATH    Read package manifest from PATH
                             (default: ``manifest.json``).
+-I file-pattern, --ignore=file-pattern     Ignore the given shell-pattern
+                                           when building the package.
+                                           The option may be repeated multiple
+                                           times to list multiple patterns to
+                                           exclude.
 --no-validate               Don't run checks from click-reviewers-tools on
                             the resulting .click file.
 
@@ -90,8 +95,13 @@
 
 Options:
 
--m PATH, --manifest=PATH    Read package manifest from PATH
-                            (default: ``manifest.json``).
+-m PATH, --manifest=PATH                   Read package manifest from PATH
+                                           (default: ``manifest.json``).
+-I file-pattern, --ignore=file-pattern     Ignore the given shell-pattern
+                                           when building the package.
+                                           The option may be repeated multiple
+                                           times to list multiple patterns to
+                                           exclude.
 
 click chroot
 ------------

=== modified file 'lib/click/database.vala'
--- lib/click/database.vala	2014-09-12 11:46:05 +0000
+++ lib/click/database.vala	2015-02-26 17:06:20 +0000
@@ -34,7 +34,11 @@
 	/**
 	 * Package manifest cannot be parsed.
 	 */
-	BAD_MANIFEST
+	BAD_MANIFEST,
+	/**
+	 * No database available for the given request
+	 */
+	INVALID
 }
 
 private string? app_pid_command = null;
@@ -602,8 +606,11 @@
 	public int size { get { return db.size; } }
 
 	public new SingleDB
-	@get (int index)
+	@get (int index) throws DatabaseError
 	{
+		if (index >= db.size)
+			throw new DatabaseError.INVALID
+                ("invalid index %i for db of size %i", index, db.size);
 		return db.get (index);
 	}
 
@@ -618,7 +625,15 @@
 	 *
 	 * The directory where changes should be written.
 	 */
-	public string overlay { get { return db.last ().root; } }
+	public string overlay 
+	{ 
+		get {
+			if (db.size == 0)
+				return "";
+			else
+				return db.last ().root;
+		}
+	}
 
 	/**
 	 * get_path:
@@ -812,21 +827,32 @@
 		return generator.to_data (null);
 	}
 
+	private void
+	ensure_db () throws Error
+	{
+		if (db.size == 0)
+			throw new DatabaseError.INVALID
+                ("no database loaded");
+	}
+
 	public void
 	maybe_remove (string package, string version) throws Error
 	{
+		ensure_db();
 		db.last ().maybe_remove (package, version);
 	}
 
 	public void
 	gc () throws Error
 	{
+		ensure_db();
 		db.last ().gc ();
 	}
 
 	public void
 	ensure_ownership () throws Error
 	{
+		ensure_db();
 		db.last ().ensure_ownership ();
 	}
 }

=== modified file 'lib/click/user.vala'
--- lib/click/user.vala	2014-12-05 15:09:48 +0000
+++ lib/click/user.vala	2015-02-26 17:06:20 +0000
@@ -658,8 +658,49 @@
 								  old_version, username);
 	}
 
-	private bool
-	stop_running_app (string package, string version)
+	private string
+	get_dbus_session_bus_env_for_current_user()
+	{
+		string euid = "%i".printf((int)(Posix.geteuid ()));
+		var dbus_session_file = Path.build_filename(
+			"/run", "user", euid, "dbus-session");
+		string session_env;
+		try {
+			FileUtils.get_contents(dbus_session_file, out session_env);
+			session_env = session_env.strip();
+		} catch (Error e) {
+			warning("Can not get the dbus session to stop app (%s)", e.message);
+		}
+		return session_env;
+	}
+
+	private bool
+	stop_single_app (string app_id)
+	{
+		// get the users dbus session when we run as root first as this
+		// is where ubuntu-app-stop listens
+		string[] envp = Environ.get();
+		envp += get_dbus_session_bus_env_for_current_user();
+
+		string[] command = {
+			"ubuntu-app-stop", app_id
+		};
+		bool res = false;
+		try {
+			int exit_status;
+			Process.spawn_sync
+			(null, command, envp,
+			 SpawnFlags.SEARCH_PATH,
+			 null, null, null, out exit_status);
+			res = Process.check_exit_status (exit_status);
+		} catch (Error e) {
+			res = false;
+		}
+		return res;
+	}
+
+	private bool
+	stop_running_apps_for_package (string package, string version)
 	{
 		var res = true;
 		if (! find_on_path ("ubuntu-app-stop"))
@@ -679,24 +720,7 @@
 		}
 		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;
-			}
-		}
+			res &= stop_single_app (@"$(package)_$(app_name)_$(version)");
 		return res;
 	}
 
@@ -718,6 +742,8 @@
 			old_version = Path.get_basename (target);
 			drop_privileges ();
 			try {
+				// stop before removing the path to the manifest
+				stop_running_apps_for_package (package, old_version);
 				unlink_force (path);
 			} finally {
 				regain_privileges ();
@@ -733,19 +759,14 @@
 			ensure_db ();
 			drop_privileges ();
 			try {
+				// stop before removing the path to the manifest
+				stop_running_apps_for_package (package, old_version);
 				symlink_force (HIDDEN_VERSION, path);
 			} finally {
 				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);
 

=== modified file 'pk-plugin/pk-plugin-click.c'
--- pk-plugin/pk-plugin-click.c	2014-09-02 08:44:39 +0000
+++ pk-plugin/pk-plugin-click.c	2015-02-26 17:06:20 +0000
@@ -637,6 +637,11 @@
 	GError *error = NULL;
 	gchar *summary = NULL;
 
+	// PK does not set a PATH, but we need one for removal
+	const gchar *old_path = g_getenv("PATH");
+	if(old_path == NULL)
+		g_setenv("PATH", DEFAULT_PATH, 0);
+
 	username = click_get_username_for_uid
 		(pk_transaction_get_uid (transaction));
 	if (!username) {
@@ -701,6 +706,9 @@
 	ret = TRUE;
 
 out:
+	if(old_path == NULL)
+		g_unsetenv("PATH");
+
 	g_free (summary);
 	if (error)
 		g_error_free (error);