click

Merge lp:click/devel into lp:click

  1. devel
  2. Merge into trunk
Proposed by Colin Watson on 2016-08-26
Status: Merged
Approved by: Colin Watson on 2016-08-28
Approved revision: 626
Merged at revision: 616
Proposed branch: lp:click/devel
Merge into: lp:click
Diff against target: 440 lines (+118/-56)
9 files modified
click/tests/gimock.py (+49/-19)
click/tests/preload.h (+0/-1)
click/tests/test_hooks.py (+11/-11)
debian/changelog (+24/-0)
debian/control (+1/-1)
debian/libclick-0.4-0.symbols (+1/-0)
lib/click/click.sym (+1/-0)
lib/click/hooks.vala (+12/-12)
preload/clickpreload.c (+19/-12)
To merge this branch: bzr merge lp:click/devel
Reviewer Review Type Date Requested Status
Łukasz Zemczak (community) Approve on 2016-08-31
click hackers 2016-08-26 Pending
Review via email: mp+304120@code.launchpad.net

Commit Message

Click 0.4.45: Preload __xmknod rather than mknod.

Description of the Change

Click 0.4.45: Preload __xmknod rather than mknod.

To post a comment you must log in.
Jamie Strandboge (jdstrand) wrote :

LGTM. Thanks!

lp:click/devel updated on 2016-08-28
627. By Colin Watson on 2016-08-28

Fix builds on xenial and earlier: build-depending on
libpackagekit-glib2-dev (...) | base-files isn't resolved properly by
apt, but we can just build-depend on libpackagekit-glib2-dev and let the
rest of the build machinery disable use of PackageKit if it's 1.0.0 or
newer.

Łukasz Zemczak (sil2100) wrote :

The mknod change will fix our yakkety touch image builds finally as currently click install is failing due to the preload library being unable to resolve mknod. I originally wasn't sure we should use __xmknod, but as Colin already mentioned we already were doing that so, well, perfect!

review: Approve

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 2016-08-28 20:59:37 +0000
4@@ -129,7 +129,7 @@
5 import xml.etree.ElementTree as etree
6
7 from click.tests.gimock_types import Stat, Stat64
8-from click.tests import get_executable
9+from click.tests import config, get_executable
10
11 # Borrowed from giscanner.girparser.
12 CORE_NS = "http://www.gtk.org/introspection/core/1.0"
13@@ -177,8 +177,6 @@
14 class GIMockTestCase(unittest.TestCase):
15 def setUp(self):
16 super(GIMockTestCase, self).setUp()
17- self._gimock_temp_dir = tempfile.mkdtemp(prefix="gimock")
18- self.addCleanup(shutil.rmtree, self._gimock_temp_dir)
19 self._preload_func_refs = []
20 self._composite_refs = []
21 self._delegate_funcs = {}
22@@ -280,6 +278,8 @@
23 preload_headers.update(headers.split(","))
24 if "GIMOCK_SUBPROCESS" in os.environ:
25 return None, rpreloads
26+ self._gimock_temp_dir = tempfile.mkdtemp(prefix="gimock")
27+ self.addCleanup(shutil.rmtree, self._gimock_temp_dir)
28 preloads_dir = os.path.join(self._gimock_temp_dir, "_preloads")
29 os.makedirs(preloads_dir)
30 c_path = os.path.join(preloads_dir, "gimockpreload.c")
31@@ -320,9 +320,16 @@
32 * resolvable until the program under test was
33 * loaded.
34 */
35+ char *err;
36 dlerror ();
37- real_%(name)s = dlsym (RTLD_NEXT, \"%(name)s\");
38- if (dlerror ()) _exit (1);
39+ real_%(name)s = dlsym (RTLD_NEXT, "%(name)s");
40+ if ((err = dlerror ()) != NULL) {
41+ fprintf (stderr,
42+ "Error getting address of symbol "
43+ "'%(name)s': %%s\\n", err);
44+ fflush (stderr);
45+ _exit (1);
46+ }
47 }
48 }
49 """) % conv, file=c)
50@@ -336,6 +343,8 @@
51 if (! delegate_%(name)s)
52 return;
53 }
54+ if (!real_%(name)s)
55+ _gimock_init_%(name)s (ctypes_%(name)s);
56 (*real_%(name)s) (%(args)s);
57 }
58 """) % conv, file=c)
59@@ -350,6 +359,8 @@
60 if (! delegate_%(name)s)
61 return %(need_strdup)s(ret);
62 }
63+ if (!real_%(name)s)
64+ _gimock_init_%(name)s (ctypes_%(name)s);
65 return (*real_%(name)s) (%(args)s);
66 }
67 """) % conv, file=c)
68@@ -363,28 +374,47 @@
69 static void __attribute__ ((constructor))
70 gimockpreload_init (void)
71 {
72+ char *err;
73 dlerror ();
74 """), file=c)
75- for info, _ in rpreloads:
76- name = info["name"]
77- print(" real_%s = dlsym (RTLD_NEXT, \"%s\");" %
78- (name, name), file=c)
79- print(" if (dlerror ()) _exit (1);", file=c)
80+ print("\n".join(" " + line for line in (dedent("""\
81+ real_%(name)s = dlsym (RTLD_NEXT, "%(name)s");
82+ if ((err = dlerror ()) != NULL) {
83+ fprintf (stderr,
84+ "Error getting address of symbol "
85+ "'%(name)s': %%s\\n", err);
86+ fflush (stderr);
87+ _exit (1);
88+ }
89+ """) % {"name": name}).split("\n")), file=c)
90 print("}", file=c)
91 if "GIMOCK_PRELOAD_DEBUG" in os.environ:
92 with open(c_path) as c:
93 print(c.read())
94 # TODO: Use libtool or similar rather than hardcoding gcc invocation.
95 lib_path = os.path.join(preloads_dir, "libgimockpreload.so")
96- cflags = subprocess.check_output([
97- "pkg-config", "--cflags", "glib-2.0", "gee-0.8", "json-glib-1.0"],
98- universal_newlines=True).rstrip("\n").split()
99- subprocess.check_call([
100- "gcc", "-O0", "-g", "-shared", "-fPIC", "-DPIC", "-I", "lib/click",
101- ] + cflags + [
102- "-Wl,-soname", "-Wl,libgimockpreload.so",
103- c_path, "-ldl", "-o", lib_path,
104- ])
105+ libraries = ["glib-2.0", "gee-0.8", "json-glib-1.0"]
106+ cflags = subprocess.check_output(
107+ ["pkg-config", "--cflags"] + libraries,
108+ universal_newlines=True).rstrip("\n").split()
109+ libs = subprocess.check_output(
110+ ["pkg-config", "--libs"] + libraries,
111+ universal_newlines=True).rstrip("\n").split()
112+ compile_cmd = [
113+ "gcc", "-O0", "-g", "-shared", "-fPIC", "-DPIC",
114+ "-I", "lib/click",
115+ "-L",
116+ os.path.join(config.abs_top_builddir, "lib", "click", ".libs"),
117+ ]
118+ compile_cmd.extend(cflags)
119+ compile_cmd.extend(["-Wl,-soname", "-Wl,libgimockpreload.so"])
120+ compile_cmd.append("-Wl,--no-as-needed")
121+ compile_cmd.append(c_path)
122+ compile_cmd.append("-lclick-0.4")
123+ compile_cmd.extend(libs)
124+ compile_cmd.append("-ldl")
125+ compile_cmd.extend(["-o", lib_path])
126+ subprocess.check_call(compile_cmd)
127 return lib_path, rpreloads
128
129 # Use as:
130
131=== modified file 'click/tests/preload.h'
132--- click/tests/preload.h 2014-11-03 08:53:50 +0000
133+++ click/tests/preload.h 2016-08-28 20:59:37 +0000
134@@ -91,7 +91,6 @@
135
136 /**
137 * click_package_install_hooks: (attributes headers=glib.h,click.h)
138-
139 * @db: (type gpointer)
140 */
141 void click_package_install_hooks (ClickDB *db, const gchar *package,
142
143=== modified file 'click/tests/test_hooks.py'
144--- click/tests/test_hooks.py 2015-10-15 11:47:17 +0000
145+++ click/tests/test_hooks.py 2016-08-28 20:59:37 +0000
146@@ -620,7 +620,7 @@
147 ) as (enter, preloads):
148 enter()
149 self._setup_hooks_dir(preloads)
150- preloads["click_get_user_home"].return_value = "/home/test-user"
151+ preloads["click_get_user_home"].return_value = b"/home/test-user"
152 os.makedirs(os.path.join(
153 self.temp_dir, "org.example.package", "1.0"))
154 user_db = Click.User.for_user(self.db, self.TEST_USER)
155@@ -646,7 +646,7 @@
156 ) as (enter, preloads):
157 enter()
158 self._setup_hooks_dir(preloads)
159- preloads["click_get_user_home"].return_value = "/home/test-user"
160+ preloads["click_get_user_home"].return_value = b"/home/test-user"
161 os.makedirs(os.path.join(
162 self.temp_dir, "org.example.package", "1.0"))
163 user_db = Click.User.for_user(self.db, self.TEST_USER)
164@@ -672,7 +672,7 @@
165 ) as (enter, preloads):
166 enter()
167 self._setup_hooks_dir(preloads)
168- preloads["click_get_user_home"].return_value = "/home/test-user"
169+ preloads["click_get_user_home"].return_value = b"/home/test-user"
170 os.makedirs(os.path.join(
171 self.temp_dir, "org.example.package", "1.0"))
172 os.makedirs(os.path.join(
173@@ -706,7 +706,7 @@
174 ) as (enter, preloads):
175 enter()
176 self._setup_hooks_dir(preloads)
177- preloads["click_get_user_home"].return_value = "/home/test-user"
178+ preloads["click_get_user_home"].return_value = b"/home/test-user"
179 symlink_path = os.path.join(
180 self.temp_dir, "org.example.package_test-app_1.0.test")
181 os.symlink("old-target", symlink_path)
182@@ -733,7 +733,7 @@
183 ) as (enter, preloads):
184 enter()
185 self._setup_hooks_dir(preloads)
186- preloads["click_get_user_home"].return_value = "/home/test-user"
187+ preloads["click_get_user_home"].return_value = b"/home/test-user"
188 self._make_hook_file(dedent("""\
189 User-Level: yes
190 Pattern: %s/${id}.test""") % self.temp_dir)
191@@ -753,7 +753,7 @@
192 enter()
193 # Don't tell click about the hooks directory yet.
194 self._setup_hooks_dir(preloads)
195- preloads["click_get_user_home"].return_value = "/home/test-user"
196+ preloads["click_get_user_home"].return_value = b"/home/test-user"
197 preloads["getpwnam"].side_effect = (
198 lambda name: self.make_pointer(Passwd(pw_uid=1, pw_gid=1)))
199 with mkfile(os.path.join(self.temp_dir, "hooks", "new.hook")) as f:
200@@ -819,7 +819,7 @@
201 enter()
202 # Don't tell click about the hooks directory yet.
203 self._setup_hooks_dir(preloads)
204- preloads["click_get_user_home"].return_value = "/home/test-user"
205+ preloads["click_get_user_home"].return_value = b"/home/test-user"
206 with mkfile(os.path.join(self.temp_dir, "hooks", "old.hook")) as f:
207 print("User-Level: yes", file=f)
208 print("Pattern: %s/${id}.old" % self.temp_dir, file=f)
209@@ -848,7 +848,7 @@
210 "click_get_hooks_dir", "click_get_user_home",
211 ) as (enter, preloads):
212 enter()
213- preloads["click_get_user_home"].return_value = "/home/test-user"
214+ preloads["click_get_user_home"].return_value = b"/home/test-user"
215 self._setup_hooks_dir(preloads)
216 with mkfile(
217 os.path.join(self.temp_dir, "hooks", "test.hook")) as f:
218@@ -894,7 +894,7 @@
219 "click_get_hooks_dir", "click_get_user_home",
220 ) as (enter, preloads):
221 enter()
222- preloads["click_get_user_home"].return_value = "/home/test-user"
223+ preloads["click_get_user_home"].return_value = b"/home/test-user"
224 self._setup_hooks_dir(preloads)
225 with mkfile(
226 os.path.join(self.temp_dir, "hooks", "test.hook")) as f:
227@@ -924,7 +924,7 @@
228 ) as (enter, preloads):
229 enter()
230 self._setup_hooks_dir(preloads)
231- preloads["click_get_user_home"].return_value = "/home/test-user"
232+ preloads["click_get_user_home"].return_value = b"/home/test-user"
233 with mkfile(os.path.join(self.temp_dir, "test.hook")) as f:
234 print("User-Level: yes", file=f)
235 print("Pattern: %s/${id}.test" % self.temp_dir, file=f)
236@@ -1143,7 +1143,7 @@
237 class TestPackageHooksValidateFramework(TestClickHookBase):
238
239 def _setup_test_env(self, preloads):
240- preloads["click_get_user_home"].return_value = "/home/test-user"
241+ preloads["click_get_user_home"].return_value = b"/home/test-user"
242 self._setup_hooks_dir(
243 preloads, os.path.join(self.temp_dir, "hooks"))
244 self._make_hook_file(dedent("""\
245
246=== modified file 'debian/changelog'
247--- debian/changelog 2016-08-11 10:50:58 +0000
248+++ debian/changelog 2016-08-28 20:59:37 +0000
249@@ -1,3 +1,27 @@
250+click (0.4.45) UNRELEASED; urgency=medium
251+
252+ * Emit more debugging information when libclickpreload/libgimockpreload
253+ fail.
254+ * Preload __xmknod rather than mknod (LP: #1615757).
255+ * Stop the test suite leaving spurious empty /tmp/gimock* directories
256+ around.
257+ * Remove stray blank line from click_package_install_hooks test preload
258+ annotations.
259+ * Link libgimockpreload properly against libclick and several other
260+ libraries, and use -Wl,--no-as-needed to stop the linker discarding
261+ references to libraries that will be used via dlsym.
262+ * Initialise real_* pointers in libgimockpreload more reliably.
263+ * Export click_get_user_home for the benefit of the test suite.
264+ * Fix click_get_user_home mocks to return bytes rather than text, avoiding
265+ obscure crashes in the test suite.
266+ * Fix builds on xenial and earlier: build-depending on
267+ libpackagekit-glib2-dev (...) | base-files isn't resolved properly by
268+ apt, but we can just build-depend on libpackagekit-glib2-dev and let the
269+ rest of the build machinery disable use of PackageKit if it's 1.0.0 or
270+ newer.
271+
272+ -- Colin Watson <cjwatson@ubuntu.com> Fri, 26 Aug 2016 18:37:49 +0100
273+
274 click (0.4.44+16.10.20160811.1-0ubuntu1) yakkety; urgency=medium
275
276 [ Colin Watson ]
277
278=== modified file 'debian/control'
279--- debian/control 2016-08-09 11:21:53 +0000
280+++ debian/control 2016-08-28 20:59:37 +0000
281@@ -3,7 +3,7 @@
282 Priority: optional
283 Maintainer: Colin Watson <cjwatson@ubuntu.com>
284 Standards-Version: 3.9.5
285-Build-Depends: debhelper (>= 9~), dh-autoreconf, intltool, python3:any (>= 3.2), python3-all:any, python3-setuptools, python3-apt, python3-debian, python3-gi, python3:any (>= 3.3) | python3-mock, pep8, python3-pep8, pyflakes, python3-sphinx, pkg-config, valac, gobject-introspection (>= 0.6.7), libgirepository1.0-dev (>= 0.6.7), libglib2.0-dev (>= 2.34), gir1.2-glib-2.0, libjson-glib-dev (>= 0.10), libgee-0.8-dev, libpackagekit-glib2-dev (>= 0.8.10) | base-files, libpackagekit-glib2-dev (<< 1.0.0) | base-files, python3-coverage, python3-six, dh-systemd (>= 1.3)
286+Build-Depends: debhelper (>= 9~), dh-autoreconf, intltool, python3:any (>= 3.2), python3-all:any, python3-setuptools, python3-apt, python3-debian, python3-gi, python3:any (>= 3.3) | python3-mock, pep8, python3-pep8, pyflakes, python3-sphinx, pkg-config, valac, gobject-introspection (>= 0.6.7), libgirepository1.0-dev (>= 0.6.7), libglib2.0-dev (>= 2.34), gir1.2-glib-2.0, libjson-glib-dev (>= 0.10), libgee-0.8-dev, libpackagekit-glib2-dev (>= 0.8.10), python3-coverage, python3-six, dh-systemd (>= 1.3)
287 Vcs-Bzr: https://code.launchpad.net/~click-hackers/click/devel
288 Vcs-Browser: http://bazaar.launchpad.net/~click-hackers/click/devel/files
289 X-Python-Version: >= 2.7
290
291=== modified file 'debian/libclick-0.4-0.symbols'
292--- debian/libclick-0.4-0.symbols 2014-03-31 10:50:20 +0000
293+++ debian/libclick-0.4-0.symbols 2016-08-28 20:59:37 +0000
294@@ -38,6 +38,7 @@
295 click_get_frameworks_dir@Base 0.4.18
296 click_get_hooks_dir@Base 0.4.17
297 click_get_umask@Base 0.4.17
298+ click_get_user_home@Base 0.4.45
299 click_hook_get_app_id@Base 0.4.17
300 click_hook_get_field@Base 0.4.17
301 click_hook_get_fields@Base 0.4.17
302
303=== modified file 'lib/click/click.sym'
304--- lib/click/click.sym 2014-03-31 10:50:20 +0000
305+++ lib/click/click.sym 2016-08-28 20:59:37 +0000
306@@ -36,6 +36,7 @@
307 click_get_frameworks_dir
308 click_get_hooks_dir
309 click_get_umask
310+click_get_user_home
311 click_hook_get_app_id
312 click_hook_get_field
313 click_hook_get_fields
314
315=== modified file 'lib/click/hooks.vala'
316--- lib/click/hooks.vala 2014-09-10 11:57:55 +0000
317+++ lib/click/hooks.vala 2016-08-28 20:59:37 +0000
318@@ -570,18 +570,6 @@
319 return @"$(short_app_id)_$(version)";
320 }
321
322- private string?
323- get_user_home (string? user_name)
324- {
325- if (user_name == null)
326- return null;
327- /* TODO: caching */
328- unowned Posix.Passwd? pw = Posix.getpwnam (user_name);
329- if (pw == null)
330- return null;
331- return pw.pw_dir;
332- }
333-
334 /**
335 * get_pattern:
336 * @package: A package name.
337@@ -1058,6 +1046,18 @@
338 }
339 }
340
341+private string?
342+get_user_home (string? user_name)
343+{
344+ if (user_name == null)
345+ return null;
346+ /* TODO: caching */
347+ unowned Posix.Passwd? pw = Posix.getpwnam (user_name);
348+ if (pw == null)
349+ return null;
350+ return pw.pw_dir;
351+}
352+
353 private Gee.TreeSet<AppHook>
354 get_app_hooks (Json.Object manifest)
355 {
356
357=== modified file 'preload/clickpreload.c'
358--- preload/clickpreload.c 2013-09-30 10:27:29 +0000
359+++ preload/clickpreload.c 2016-08-28 20:59:37 +0000
360@@ -47,10 +47,10 @@
361 static int (*libc_link) (const char *, const char *) = (void *) 0;
362 static int (*libc_mkdir) (const char *, mode_t) = (void *) 0;
363 static int (*libc_mkfifo) (const char *, mode_t) = (void *) 0;
364-static int (*libc_mknod) (const char *, mode_t, dev_t) = (void *) 0;
365 static int (*libc_open) (const char *, int, mode_t) = (void *) 0;
366 static int (*libc_open64) (const char *, int, mode_t) = (void *) 0;
367 static int (*libc_symlink) (const char *, const char *) = (void *) 0;
368+static int (*libc___xmknod) (int, const char *, mode_t, dev_t *) = (void *) 0;
369 static int (*libc___xstat) (int, const char *, struct stat *) = (void *) 0;
370 static int (*libc___xstat64) (int, const char *, struct stat64 *) = (void *) 0;
371
372@@ -64,9 +64,15 @@
373
374 #define GET_NEXT_SYMBOL(name) \
375 do { \
376+ char *err; \
377 libc_##name = dlsym (RTLD_NEXT, #name); \
378- if (dlerror ()) \
379+ if ((err = dlerror ()) != NULL) { \
380+ fprintf (stderr, \
381+ "Error getting address of symbol '" #name "': %s\n", \
382+ err); \
383+ fflush (stderr); \
384 _exit (1); \
385+ } \
386 } while (0)
387
388 static void __attribute__ ((constructor)) clickpreload_init (void)
389@@ -89,10 +95,10 @@
390 GET_NEXT_SYMBOL (link);
391 GET_NEXT_SYMBOL (mkdir);
392 GET_NEXT_SYMBOL (mkfifo);
393- GET_NEXT_SYMBOL (mknod);
394 GET_NEXT_SYMBOL (open);
395 GET_NEXT_SYMBOL (open64);
396 GET_NEXT_SYMBOL (symlink);
397+ GET_NEXT_SYMBOL (__xmknod);
398 GET_NEXT_SYMBOL (__xstat);
399 GET_NEXT_SYMBOL (__xstat64);
400
401@@ -236,6 +242,7 @@
402 fprintf (stderr,
403 "Sandbox failure: 'click install' not permitted to %s '%s'\n",
404 verb, pathname);
405+ fflush (stderr);
406 exit (1);
407 }
408
409@@ -266,15 +273,6 @@
410 return (*libc_mkfifo) (pathname, mode);
411 }
412
413-int mknod (const char *pathname, mode_t mode, dev_t dev)
414-{
415- if (!libc_mknod)
416- clickpreload_init (); /* also needed for base_path, base_path_len */
417-
418- clickpreload_assert_path_in_instdir ("mknod", pathname);
419- return (*libc_mknod) (pathname, mode, dev);
420-}
421-
422 int symlink (const char *oldpath, const char *newpath)
423 {
424 if (!libc_symlink)
425@@ -393,6 +391,15 @@
426 return ret;
427 }
428
429+int __xmknod (int ver, const char *pathname, mode_t mode, dev_t *dev)
430+{
431+ if (!libc___xmknod)
432+ clickpreload_init (); /* also needed for base_path, base_path_len */
433+
434+ clickpreload_assert_path_in_instdir ("mknod", pathname);
435+ return (*libc___xmknod) (ver, pathname, mode, dev);
436+}
437+
438 int __xstat (int ver, const char *pathname, struct stat *buf)
439 {
440 if (!libc___xstat)

Subscribers

People subscribed via source and target branches

to all changes: