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

Proposed by Michael Vogt on 2014-09-29
Status: Merged
Merged at revision: 525
Proposed branch: lp:~mvo/click/lp1232130-kill-on-remove
Merge into: lp:click/devel
Diff against target: 140 lines (+86/-2)
3 files modified
click/tests/helpers.py (+8/-0)
click/tests/test_user.py (+28/-2)
lib/click/user.vala (+50/-0)
To merge this branch: bzr merge lp:~mvo/click/lp1232130-kill-on-remove
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration 2014-09-29 Approve on 2014-09-29
Colin Watson 2014-09-29 Approve on 2014-09-29
Review via email: mp+236298@code.launchpad.net

Commit message

Stop apps if necessary when uninstalling them.

Description of the change

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).

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'click/tests/helpers.py'
2--- click/tests/helpers.py 2014-06-23 21:14:06 +0000
3+++ click/tests/helpers.py 2014-09-29 11:15:22 +0000
4@@ -326,3 +326,11 @@
5 def touch(path):
6 with mkfile(path, mode="a"):
7 pass
8+
9+
10+def make_file_with_content(filename, content, mode=0o644):
11+ """Create a file with the given content and mode"""
12+ Click.ensuredir(os.path.dirname(filename))
13+ with open(filename, "w") as f:
14+ f.write(content)
15+ os.chmod(filename, 0o755)
16
17=== modified file 'click/tests/test_user.py'
18--- click/tests/test_user.py 2014-07-09 12:56:21 +0000
19+++ click/tests/test_user.py 2014-09-29 11:15:22 +0000
20@@ -26,12 +26,18 @@
21 import json
22 import os
23 import shutil
24+from textwrap import dedent
25
26 from gi.repository import Click, GLib
27
28 from click.json_helpers import json_array_to_python, json_object_to_python
29 from click.tests.gimock_types import Passwd
30-from click.tests.helpers import TestCase, mkfile, touch
31+from click.tests.helpers import (
32+ TestCase,
33+ mkfile,
34+ make_file_with_content,
35+ touch,
36+)
37
38
39 class TestClickUser(TestCase):
40@@ -53,7 +59,8 @@
41 a_1_0 = os.path.join(self.temp_dir, "custom", "a", "1.0")
42 os.makedirs(a_1_0)
43 with mkfile(os.path.join(a_1_0, ".click", "info", "a.manifest")) as m:
44- json.dump({"name": "a", "version": "1.0"}, m)
45+ json.dump({"name": "a", "version": "1.0",
46+ "hooks": {"a-app": {}}}, m)
47 b_2_0 = os.path.join(self.temp_dir, "custom", "b", "2.0")
48 os.makedirs(b_2_0)
49 with mkfile(os.path.join(b_2_0, ".click", "info", "b.manifest")) as m:
50@@ -549,3 +556,22 @@
51 self.assertEqual("2.0", registry.get_version("b"))
52 self.assertEqual(b_overlay, registry.get_path("b"))
53 self.assertTrue(registry.is_removable("b"))
54+
55+ def test_app_stops_on_remove(self):
56+ fake_app_stop = os.path.join(self.temp_dir, "bin", "ubuntu-app-stop")
57+ fake_app_stop_output = os.path.join(self.temp_dir, "fake-app-stop.out")
58+ fake_app_stop_content = dedent("""\
59+ #!/bin/sh
60+ echo "$@" >> %s
61+ """ % fake_app_stop_output)
62+ make_file_with_content(fake_app_stop, fake_app_stop_content, 0o755)
63+ # its ok to modify env here, click.helpers.TestCase will take care
64+ # of it
65+ os.environ["PATH"] = "%s:%s" % (
66+ os.path.dirname(fake_app_stop), os.environ["PATH"])
67+ # get a app with manifest etc all
68+ _, registry = self._setUpMultiDB()
69+ registry.remove("a")
70+ # ensure that stop was called with the right app
71+ with open(fake_app_stop_output) as f:
72+ self.assertEqual("a_a-app_1.1", f.read().strip())
73
74=== modified file 'lib/click/user.vala'
75--- lib/click/user.vala 2014-07-11 17:16:35 +0000
76+++ lib/click/user.vala 2014-09-29 11:15:22 +0000
77@@ -598,6 +598,48 @@
78 old_version, version, name);
79 }
80
81+ private bool
82+ stop_running_app (string package, string version)
83+ {
84+ var res = true;
85+ if (! find_on_path ("ubuntu-app-stop"))
86+ return false;
87+
88+ Json.Object manifest;
89+ try {
90+ manifest = get_manifest (package);
91+ } catch (Error e) {
92+ warning ("Can not get manifest for %s", package);
93+ return false;
94+ }
95+
96+ if (! manifest.has_member ("hooks")) {
97+ warning ("No hooks in manifest %s", package);
98+ return false;
99+ }
100+ var hooks = manifest.get_object_member ("hooks");
101+ foreach (unowned string app_name in
102+ hooks.get_members ())
103+ {
104+ // FIXME: move this into a "stop_single_app" helper
105+ string[] command = {
106+ "ubuntu-app-stop",
107+ @"$(package)_$(app_name)_$(version)"
108+ };
109+ try {
110+ int exit_status;
111+ Process.spawn_sync (null, command, null,
112+ SpawnFlags.SEARCH_PATH |
113+ SpawnFlags.STDOUT_TO_DEV_NULL,
114+ null, null, null, out exit_status);
115+ res &= Process.check_exit_status (exit_status);
116+ } catch (Error e) {
117+ res &= false;
118+ }
119+ }
120+ return res;
121+ }
122+
123 /**
124 * remove:
125 * @package: A package name.
126@@ -636,6 +678,14 @@
127 regain_privileges ();
128 }
129 }
130+
131+ drop_privileges ();
132+ try {
133+ stop_running_app (package, old_version);
134+ } finally {
135+ regain_privileges ();
136+ }
137+
138 if (! is_pseudo_user)
139 package_remove_hooks (db, package, old_version, name);
140 }

Subscribers

People subscribed via source and target branches

to all changes: