Merge lp:~mvo/click/lp1245826-multiple-hookfiles into lp:click/devel

Proposed by Michael Vogt on 2014-09-22
Status: Needs review
Proposed branch: lp:~mvo/click/lp1245826-multiple-hookfiles
Merge into: lp:click/devel
Diff against target: 299 lines (+103/-32)
3 files modified
click/tests/gimock.py (+2/-1)
click/tests/test_hooks.py (+46/-2)
lib/click/hooks.vala (+55/-29)
To merge this branch: bzr merge lp:~mvo/click/lp1245826-multiple-hookfiles
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing on 2014-09-30
click hackers 2014-09-22 Pending
Review via email: mp+235474@code.launchpad.net

Commit message

Add support for multiple click hook files.

Description of the change

Add support for multiple click hook files.

This is based on the work of lp:~mardy/click/lp1245826. Feedback on the approach appreciated, the actual code needs a refactor to get rid of the duplication of the relative_path reading code.

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

FAILED: Continuous integration, rev:524
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/lp1245826-multiple-hookfiles/+merge/235474/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/click-devel-ci/69/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/click-devel-utopic-amd64-ci/71/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/click-devel-utopic-armhf-ci/69/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/click-devel-utopic-i386-ci/69/console

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

review: Needs Fixing (continuous-integration)
525. By Michael Vogt on 2014-09-23

refactor duplicated code into get_relative_paths_for_hookname()

PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:525
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/lp1245826-multiple-hookfiles/+merge/235474/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/click-devel-ci/70/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/click-devel-utopic-amd64-ci/72/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/click-devel-utopic-armhf-ci/70/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/click-devel-utopic-i386-ci/70/console

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

review: Needs Fixing (continuous-integration)
526. By Michael Vogt on 2014-09-30

fix pep8 failure

PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:526
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/lp1245826-multiple-hookfiles/+merge/235474/+edit-commit-message

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

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

review: Needs Fixing (continuous-integration)

Unmerged revisions

526. By Michael Vogt on 2014-09-30

fix pep8 failure

525. By Michael Vogt on 2014-09-23

refactor duplicated code into get_relative_paths_for_hookname()

524. By Michael Vogt on 2014-09-22

add support for relative_path in get_app_hooks

523. By Michael Vogt on 2014-09-22

fix remove and add test

522. By Michael Vogt on 2014-09-22

Add support for multiple hook files

This allows writing hook files like:
"""
 Pattern: %s/${id}_${basename}
"""

and manifest with:
"""
hooks": {
  "app": {
     "service": ["target-1","target-2"]
  }
}
"""

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'click/tests/gimock.py'
2--- click/tests/gimock.py 2014-06-23 21:14:06 +0000
3+++ click/tests/gimock.py 2014-09-30 05:57:11 +0000
4@@ -443,7 +443,8 @@
5 env["LD_PRELOAD"] = lib_path
6 subp = subprocess.Popen(args, close_fds=False, env=env,
7 stdout=subprocess.PIPE,
8- stderr=subprocess.PIPE)
9+ stderr=subprocess.PIPE,
10+ universal_newlines=True)
11 os.close(wfd)
12 reader = os.fdopen(rfd, "rb")
13 stdout, stderr = subp.communicate()
14
15=== modified file 'click/tests/test_hooks.py'
16--- click/tests/test_hooks.py 2014-07-11 17:20:51 +0000
17+++ click/tests/test_hooks.py 2014-09-30 05:57:11 +0000
18@@ -424,6 +424,31 @@
19 os.path.join(self.temp_dir, "test-2", "2.0", "target-2"),
20 os.readlink(path_2))
21
22+ def test_install_hook_with_multiple_files(self):
23+ with self.run_in_subprocess(
24+ "click_get_hooks_dir") as (enter, preloads):
25+ enter()
26+ self._setup_hooks_dir(
27+ preloads, hooks_dir=os.path.join(self.temp_dir, "hooks"))
28+ self._make_hook_file(
29+ "Pattern: %s/${id}_${basename}.new" % self.temp_dir,
30+ hookname="new")
31+ self._make_installed_click("test-1", "1.0", json_data={
32+ "hooks": {"test1-app": {"new": ["target-1","target-2"]}}})
33+ hook = Click.Hook.open(self.db, "new")
34+ hook.install(user_name=None)
35+ for hook_file in ("target-1", "target-2"):
36+ path = os.path.join(
37+ self.temp_dir, "test-1_test1-app_1.0_%s.new" % hook_file)
38+ expected_path = os.path.join(
39+ self.temp_dir, "test-1", "1.0", hook_file)
40+ self.assertEqual(expected_path, os.readlink(path))
41+ hook.remove(user_name=None)
42+ for hook_file in ("target-1", "target-2"):
43+ path = os.path.join(
44+ self.temp_dir, "test-1_test1-app_1.0_%s.new" % hook_file)
45+ self.assertEqual(os.path.lexists(path), False)
46+
47 def test_remove(self):
48 with self.run_in_subprocess(
49 "click_get_hooks_dir") as (enter, preloads):
50@@ -1114,6 +1139,10 @@
51 print("Pattern: %s/yelp/other-${id}.txt" % self.temp_dir,
52 file=f)
53 print("Hook-Name: yelp", file=f)
54+ with mkfile(os.path.join(hooks_dir, "service.hook")) as f:
55+ print("Pattern: %s/service/${id}_${basename}.txt" % self.temp_dir,
56+ file=f)
57+ print("Hook-Name: service", file=f)
58 os.mkdir(os.path.join(self.temp_dir, "unity"))
59 unity_path = os.path.join(
60 self.temp_dir, "unity", "test_app_1.0.scope")
61@@ -1125,18 +1154,33 @@
62 yelp_other_path = os.path.join(
63 self.temp_dir, "yelp", "other-test_app_1.0.txt")
64 os.symlink("dummy", yelp_other_path)
65+ os.mkdir(os.path.join(self.temp_dir, "service"))
66+ service1_path = os.path.join(
67+ self.temp_dir, "service", "test_app_1.0_service1.txt")
68+ os.symlink("dummy", service1_path)
69+ service2_path = os.path.join(
70+ self.temp_dir, "service", "test_app_1.0_service2.txt")
71+ os.symlink("dummy", service2_path)
72 package_dir = os.path.join(self.temp_dir, "test")
73 with mkfile(os.path.join(
74 package_dir, "1.0", ".click", "info",
75 "test.manifest")) as f:
76 json.dump(
77- {"hooks": {
78- "app": {"yelp": "foo.txt", "unity": "foo.scope"}}
79+ {"hooks":
80+ {
81+ "app": {
82+ "yelp": "foo.txt",
83+ "unity": "foo.scope",
84+ "service": ["service1", "service2"],
85+ },
86+ }
87 }, f)
88 Click.package_remove_hooks(self.db, "test", "1.0", user_name=None)
89 self.assertFalse(os.path.lexists(unity_path))
90 self.assertFalse(os.path.lexists(yelp_docs_path))
91 self.assertFalse(os.path.lexists(yelp_other_path))
92+ self.assertFalse(os.path.lexists(service1_path))
93+ self.assertFalse(os.path.lexists(service2_path))
94
95
96 class TestPackageHooksValidateFramework(TestClickHookBase):
97
98=== modified file 'lib/click/hooks.vala'
99--- lib/click/hooks.vala 2014-09-10 11:57:55 +0000
100+++ lib/click/hooks.vala 2014-09-30 05:57:11 +0000
101@@ -227,23 +227,24 @@
102 Gee.Comparable<AppHook> {
103 public string app_name { get; construct; }
104 public string hook_name { get; construct; }
105+ public string relative_path { get; construct; }
106
107 public
108- AppHook (string app_name, string hook_name)
109+ AppHook (string app_name, string hook_name, string? relative_path = null)
110 {
111- Object (app_name: app_name, hook_name: hook_name);
112+ Object (app_name: app_name, hook_name: hook_name, relative_path: relative_path);
113 }
114
115 public uint
116 hash ()
117 {
118- return app_name.hash () ^ hook_name.hash ();
119+ return app_name.hash () ^ hook_name.hash () ^ relative_path.hash();
120 }
121
122 public bool
123 equal_to (AppHook obj)
124 {
125- return app_name == obj.app_name && hook_name == obj.hook_name;
126+ return app_name == obj.app_name && hook_name == obj.hook_name && relative_path == obj.relative_path;
127 }
128
129 public int
130@@ -252,7 +253,10 @@
131 var ret = strcmp (app_name, obj.app_name);
132 if (ret != 0)
133 return ret;
134- return strcmp (hook_name, obj.hook_name);
135+ ret = strcmp (hook_name, obj.hook_name);
136+ if (ret != 0)
137+ return ret;
138+ return strcmp (relative_path, obj.relative_path);
139 }
140 }
141
142@@ -588,10 +592,11 @@
143 * @version: A version string.
144 * @app_name: An application name.
145 * @user_name: (allow-none): A user name, or null.
146+ * @base_name: (allow-none): A base name, or null.
147 */
148 public string
149 get_pattern (string package, string version, string app_name,
150- string? user_name = null) throws HooksError
151+ string? user_name = null, string? base_name = "") throws HooksError
152 {
153 var builder = new VariantBuilder (new VariantType ("a{sms}"));
154 var app_id = get_app_id (package, version, app_name);
155@@ -600,9 +605,9 @@
156 builder.add ("{sms}", "id", app_id);
157 builder.add ("{sms}", "user", user_name);
158 builder.add ("{sms}", "home", user_home);
159+ builder.add ("{sms}", "basename", base_name);
160 if (is_single_version) {
161- var short_app_id = get_short_app_id (package,
162- app_name);
163+ var short_app_id = get_short_app_id (package, app_name);
164 builder.add ("{sms}", "short-id", short_app_id);
165 }
166 var ret = pattern_format (pattern, builder.end ());
167@@ -799,7 +804,8 @@
168 else
169 path = db.get_path (package, version);
170 var target = Path.build_filename (path, relative_path);
171- var link = get_pattern (package, version, app_name, user_name);
172+ var link = get_pattern (package, version, app_name, user_name,
173+ Path.get_basename (relative_path));
174 if (is_symlink (link) && FileUtils.read_link (link) == target)
175 return;
176 ensuredir (Path.get_dirname (link));
177@@ -862,10 +868,10 @@
178 */
179 public void
180 remove_package (string package, string version, string app_name,
181- string? user_name = null) throws Error
182+ string? user_name = null, string? base_name = null) throws Error
183 {
184 unlink_force (get_pattern
185- (package, version, app_name, user_name));
186+ (package, version, app_name, user_name, base_name));
187 run_commands (user_name);
188 }
189
190@@ -953,17 +959,14 @@
191 var manifest_hooks = read_manifest_hooks
192 (db, unpacked.package, unpacked.version);
193 foreach (var app_name in manifest_hooks.get_members ()) {
194- var hooks = manifest_hooks.get_object_member
195- (app_name);
196- if (hooks.has_member (hook_name)) {
197- var relative_path = hooks.get_string_member
198- (hook_name);
199- ret.prepend (new RelevantApp
200- (unpacked.package,
201- unpacked.version, app_name,
202- unpacked.user_name,
203- relative_path));
204- }
205+ foreach (var relative_path in get_relative_paths_for_hookname(manifest_hooks, app_name, hook_name))
206+ {
207+ ret.prepend (new RelevantApp
208+ (unpacked.package,
209+ unpacked.version, app_name,
210+ unpacked.user_name,
211+ relative_path));
212+ }
213 }
214 }
215 ret.reverse ();
216@@ -998,7 +1001,7 @@
217 {
218 foreach (var app in get_relevant_apps (user_name))
219 remove_package (app.package, app.version, app.app_name,
220- app.user_name);
221+ app.user_name, app.relative_path);
222 }
223
224 /**
225@@ -1058,6 +1061,28 @@
226 }
227 }
228
229+private List<string>
230+get_relative_paths_for_hookname(Json.Object manifest, string app_name, string hook_name) throws Error
231+{
232+ var ret = new List<string> ();
233+ var hooks = manifest.get_object_member (app_name);
234+ if (!hooks.has_member (hook_name))
235+ return ret;
236+
237+ var node = hooks.get_member(hook_name);
238+ var value_type = node.get_value_type();
239+ // support both single string and list for hook targets in manifest
240+ if (value_type.name() == "gchararray")
241+ ret.append (node.get_string());
242+ else {
243+ var array = node.get_array ();
244+ foreach(var elm in array.get_elements ()) {
245+ ret.append (elm.get_string ());
246+ }
247+ }
248+ return ret;
249+}
250+
251 private Gee.TreeSet<AppHook>
252 get_app_hooks (Json.Object manifest)
253 {
254@@ -1065,7 +1090,8 @@
255 foreach (var app_name in manifest.get_members ()) {
256 var hooks = manifest.get_object_member (app_name);
257 foreach (var hook_name in hooks.get_members ())
258- items.add (new AppHook (app_name, hook_name));
259+ foreach (var relative_path in get_relative_paths_for_hookname(manifest, app_name, hook_name))
260+ items.add (new AppHook (app_name, hook_name, relative_path));
261 }
262 return items;
263 }
264@@ -1105,7 +1131,7 @@
265 if (! hook.is_single_version)
266 continue;
267 hook.remove_package (package, old_version,
268- app_hook.app_name, user_name);
269+ app_hook.app_name, user_name, app_hook.relative_path);
270 }
271 }
272
273@@ -1116,14 +1142,14 @@
274 var hook_names = app_hooks.get_members ();
275 hook_names.sort (strcmp);
276 foreach (var hook_name in hook_names) {
277- var relative_path = app_hooks.get_string_member
278- (hook_name);
279- foreach (var hook in Hook.open_all (db, hook_name)) {
280+ foreach (var relative_path in get_relative_paths_for_hookname (new_manifest, app_name, hook_name)) {
281+ foreach (var hook in Hook.open_all (db, hook_name)) {
282 if (hook.is_user_level != (user_name != null))
283 continue;
284 hook.install_package (package, new_version,
285 app_name, relative_path,
286 user_name);
287+ }
288 }
289 }
290 }
291@@ -1152,7 +1178,7 @@
292 if (hook.is_user_level != (user_name != null))
293 continue;
294 hook.remove_package (package, old_version,
295- app_hook.app_name, user_name);
296+ app_hook.app_name, user_name, app_hook.relative_path);
297 }
298 }
299 }

Subscribers

People subscribed via source and target branches

to all changes: