Merge lp:~mardy/click/lp1245826 into lp:click

Proposed by Alberto Mardegan on 2014-02-04
Status: Rejected
Rejected by: Michael Vogt on 2015-02-23
Proposed branch: lp:~mardy/click/lp1245826
Merge into: lp:click
Diff against target: 203 lines (+112/-6)
3 files modified
click/hooks.py (+12/-5)
click/tests/test_hooks.py (+85/-0)
doc/hooks.rst (+15/-1)
To merge this branch: bzr merge lp:~mardy/click/lp1245826
Reviewer Review Type Date Requested Status
Alberto Mardegan (community) Disapprove on 2014-09-29
PS Jenkins bot (community) continuous-integration Needs Fixing on 2014-06-16
Colin Watson 2014-02-04 Needs Fixing on 2014-03-18
Review via email: mp+204674@code.launchpad.net

Commit message

Support multiple source files per hook

Description of the change

WIP: documentation is missing

This is the implementation for bug 1245826.

To post a comment you must log in.
lp:~mardy/click/lp1245826 updated on 2014-02-12
337. By Alberto Mardegan on 2014-02-12

Add documentation

Colin Watson (cjwatson) wrote :

This looks basically good, and at least I'm fine with the changes to the hooks specification; thanks, and sorry for the delay on this.

However, the implementation will need to be ported to libclick. I also suspect that the handling of removals isn't right - it looks to me as though any hooks whose patterns include ${basename} won't have their symlinks properly removed. We can probably deal with this using the possible_expansion logic.

Given how long I've kept you waiting, I think it's sensible for me to just merge this into libclick myself and figure out tests sufficient to exercise my suspicion above. I'll try to get this done today.

review: Needs Fixing
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Michael Vogt (mvo) wrote :

I ported the change to the new libclick vala code in lp:~mvo/click/lp1245826-multiple-hookfiles. This MP here can probably be closed.

Alberto Mardegan (mardy) wrote :

Indeed, I'll delete it.

Alberto Mardegan (mardy) :
review: Disapprove

Unmerged revisions

337. By Alberto Mardegan on 2014-02-12

Add documentation

336. By Alberto Mardegan on 2014-02-04

Support multiple files per hook

335. By Alberto Mardegan on 2014-02-04

Add "basename" variable to hook Pattern

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'click/hooks.py'
2--- click/hooks.py 2014-01-22 12:46:12 +0000
3+++ click/hooks.py 2014-02-12 10:51:58 +0000
4@@ -176,7 +176,7 @@
5 # TODO: caching
6 return pwd.getpwnam(user).pw_dir
7
8- def pattern(self, package, version, app_name, user=None):
9+ def pattern(self, package, version, app_name, user=None, basename=None):
10 app_id = self.app_id(package, version, app_name)
11 kwargs = {
12 "id": app_id,
13@@ -185,6 +185,8 @@
14 }
15 if self.single_version:
16 kwargs["short-id"] = self.short_app_id(package, app_name)
17+ if basename:
18+ kwargs["basename"] = basename
19 return self._formatter.format(self["pattern"], **kwargs).rstrip(os.sep)
20
21 def _drop_privileges(self, username):
22@@ -252,7 +254,8 @@
23 else:
24 target = os.path.join(
25 self.db.path(package, version), relative_path)
26- link = self.pattern(package, version, app_name, user=user)
27+ link = self.pattern(package, version, app_name, user=user,
28+ basename=os.path.basename(relative_path))
29 if not os.path.islink(link) or os.readlink(link) != target:
30 osextras.ensuredir(os.path.dirname(link))
31 osextras.symlink_force(target, link)
32@@ -317,9 +320,13 @@
33 manifest = _read_manifest_hooks(self.db, package, version)
34 for app_name, hooks in manifest.items():
35 if self.hook_name in hooks:
36- yield (
37- package, version, app_name, user_name,
38- hooks[self.hook_name])
39+ source_files = hooks[self.hook_name]
40+ if isinstance(source_files, unicode):
41+ source_files = [ source_files ]
42+ for source_file in source_files:
43+ yield (
44+ package, version, app_name, user_name,
45+ source_file)
46
47 def install(self, user=None):
48 for package, version, app_name, user_name, relative_path in (
49
50=== modified file 'click/tests/test_hooks.py'
51--- click/tests/test_hooks.py 2014-01-22 12:46:12 +0000
52+++ click/tests/test_hooks.py 2014-02-12 10:51:58 +0000
53@@ -388,6 +388,24 @@
54 ValueError, hook.app_id, "package", "0.1", "app/name")
55
56 @mock.patch("pwd.getpwnam")
57+ def test_basename(self, mock_getpwnam):
58+ class MockPasswd:
59+ def __init__(self, pw_dir):
60+ self.pw_dir = pw_dir
61+
62+ mock_getpwnam.return_value = MockPasswd(pw_dir="/mock")
63+ with mkfile(os.path.join(self.temp_dir, "test.hook")) as f:
64+ print(dedent("""\
65+ User-Level: yes
66+ Pattern: ${home}/.local/share/test/${short-id}_${basename}
67+ """), file=f)
68+ with temp_hooks_dir(self.temp_dir):
69+ hook = ClickHook.open(self.db, "test")
70+ self.assertEqual(
71+ "/mock/.local/share/test/package_app-name_file.test",
72+ hook.pattern("package", "0.1", "app-name", user="mock", basename="file.test"))
73+
74+ @mock.patch("pwd.getpwnam")
75 def test_short_id_valid(self, mock_getpwnam):
76 class MockPasswd:
77 def __init__(self, pw_dir):
78@@ -513,6 +531,39 @@
79 self.assertEqual(target_path, os.readlink(symlink_path))
80
81 @mock.patch("click.hooks.ClickHook._user_home")
82+ def test_install_package_basename(self, mock_user_home):
83+ mock_user_home.return_value = "/home/test-user"
84+ with temp_hooks_dir(self.temp_dir):
85+ os.makedirs(os.path.join(
86+ self.temp_dir, "org.example.package", "1.0"))
87+ user_db = ClickUser(self.db, user="test-user")
88+ user_db.set_version("org.example.package", "1.0")
89+ with mkfile(os.path.join(self.temp_dir, "test.hook")) as f:
90+ print("User-Level: yes", file=f)
91+ print("Pattern: %s/${id}_${basename}" % self.temp_dir, file=f)
92+ hook = ClickHook.open(self.db, "test")
93+ hook.install_package(
94+ "org.example.package", "1.0", "test-app", "foo",
95+ user="test-user")
96+ symlink_path = os.path.join(
97+ self.temp_dir, "org.example.package_test-app_1.0_foo")
98+ target_path = os.path.join(
99+ self.temp_dir, ".click", "users", "test-user",
100+ "org.example.package", "foo")
101+ self.assertTrue(os.path.islink(symlink_path))
102+ self.assertEqual(target_path, os.readlink(symlink_path))
103+ hook.install_package(
104+ "org.example.package", "1.0", "test-app", "assets/icon.png",
105+ user="test-user")
106+ symlink_path = os.path.join(
107+ self.temp_dir, "org.example.package_test-app_1.0_icon.png")
108+ target_path = os.path.join(
109+ self.temp_dir, ".click", "users", "test-user",
110+ "org.example.package", "assets", "icon.png")
111+ self.assertTrue(os.path.islink(symlink_path))
112+ self.assertEqual(target_path, os.readlink(symlink_path))
113+
114+ @mock.patch("click.hooks.ClickHook._user_home")
115 def test_upgrade(self, mock_user_home):
116 mock_user_home.return_value = "/home/test-user"
117 symlink_path = os.path.join(
118@@ -552,6 +603,40 @@
119 self.assertFalse(os.path.exists(symlink_path))
120
121 @mock.patch("click.hooks.ClickHook._user_home")
122+ def test_install_many_files(self, mock_user_home):
123+ mock_user_home.return_value = "/home/test-user"
124+ with mkfile(os.path.join(self.temp_dir, "hooks", "new.hook")) as f:
125+ print("User-Level: yes", file=f)
126+ print("Pattern: %s/${id}_${basename}" % self.temp_dir, file=f)
127+ user_db = ClickUser(self.db, user="test-user")
128+ with mkfile_utf8(os.path.join(
129+ self.temp_dir, "test-1", "1.0", ".click", "info",
130+ "test-1.manifest")) as f:
131+ json.dump({
132+ "maintainer":
133+ b"Unic\xc3\xb3de <unicode@example.org>".decode("UTF-8"),
134+ "hooks": {"test1-app": {"new": [ "target-1", "dir/target-2" ]}},
135+ }, f, ensure_ascii=False)
136+ user_db.set_version("test-1", "1.0")
137+ with temp_hooks_dir(os.path.join(self.temp_dir, "hooks")):
138+ hook = ClickHook.open(self.db, "new")
139+ hook.install()
140+ path_1 = os.path.join(self.temp_dir, "test-1_test1-app_1.0_target-1")
141+ self.assertTrue(os.path.lexists(path_1))
142+ self.assertEqual(
143+ os.path.join(
144+ self.temp_dir, ".click", "users", "test-user", "test-1",
145+ "target-1"),
146+ os.readlink(path_1))
147+ path_2 = os.path.join(self.temp_dir, "test-1_test1-app_1.0_target-2")
148+ self.assertTrue(os.path.lexists(path_2))
149+ self.assertEqual(
150+ os.path.join(
151+ self.temp_dir, ".click", "users", "test-user", "test-1",
152+ "dir", "target-2"),
153+ os.readlink(path_2))
154+
155+ @mock.patch("click.hooks.ClickHook._user_home")
156 def test_install(self, mock_user_home):
157 mock_user_home.return_value = "/home/test-user"
158 with mkfile(os.path.join(self.temp_dir, "hooks", "new.hook")) as f:
159
160=== modified file 'doc/hooks.rst'
161--- doc/hooks.rst 2014-01-22 11:43:39 +0000
162+++ doc/hooks.rst 2014-02-12 10:51:58 +0000
163@@ -113,6 +113,10 @@
164 ``${home}``
165 The user's home directory (user-level hooks only).
166
167+ ``${basename}``
168+ The name of the file being processed by the hook, with the path
169+ component removed.
170+
171 ``$$``
172 The character '``$``'.
173
174@@ -192,7 +196,9 @@
175 ``*.hook`` files with matching hook-names (see ``Hook-Name`` above). The
176 paths are relative to the directory where the Click package is unpacked,
177 and are used as symlink targets by the package manager when creating
178- symlinks according to the ``Pattern`` field in ``*.hook`` files.
179+ symlinks according to the ``Pattern`` field in ``*.hook`` files. Paths can
180+ consist of a string identifying a single file, or a list of strings if the
181+ hook must be applied to multiple files.
182
183 * There is a dh_click program which installs the ``*.hook`` files in system
184 packages and adds maintainer script fragments to cause click to catch up
185@@ -215,11 +221,19 @@
186 Exec: click desktophook
187 Hook-Name: desktop
188
189+ /usr/share/click/hooks/account-service.hook:
190+ Pattern: ${home}/.local/share/accounts/services/${short-id}_${basename}
191+ User-Level: yes
192+
193 com.ubuntu.example/manifest.json:
194 "hooks": {
195 "example-app": {
196 "apparmor": "apparmor/example-app.json",
197 "desktop": "example-app.desktop",
198+ "account-service": [
199+ "services/example-mail.service",
200+ "services/example-chat.service"
201+ ],
202 }
203 }
204

Subscribers

People subscribed via source and target branches