|Summary:||add new pkg-config libudev-hwdb|
|Product:||systemd||Reporter:||Allison Lortie (desrt) <desrt>|
|Status:||RESOLVED WONTFIX||QA Contact:||systemd-bugs|
|Priority:||medium||CC:||bugzilla, kay, kwm|
|i915 platform:||i915 features:|
[PATCH 1/5] udev: drop 'udev' field from udev_list struct
[PATCH 2/5] udev: drop 'udev' paramter from udev_list_init()
[PATCH 3/5] udev: udev_hwdb_new: document unused parameter
[PATCH 4/5] udev: split out new libudev-hwdb.h header
[PATCH 5/5] udev: add libudev-hwdb.pc as copy of libudev.pc
Description Allison Lortie (desrt) 2013-12-10 12:21:28 UTC
The hwdb functionality of udev actually has very little to do with udev itself, in the sense that it would be easily possible to have this in a very simple standalone library. Some people (colord) use the hwdb database as a hard dependency, but do not require any of the rest of udev. It would be nice if they could depend only on the hwdb component, so that other platforms could implement the udev_hwdb API without having all of udev. The attached patchset does some minor restructuring and introduces a new libudev-hwdb.pc and libudev-hwdb.h which can be used as an entry to the udev-hwdb functionality, without the need to use udev proper. The changes are minimal -- we still only build one library, and the new .pc file links this library. Having a separate .pc file and header, however, will allow colord to depend only on the hwdb components. It would also be possible to skip the last two patches in the series and have colord declare a dependency on udev but only use udev_hwdb, but that would put 3rd party implementations in the awkward position of having to claim to be 'udev' while only implementing a small fraction of the API. In fact, we could skip everything here entirely and have colord just start passing NULL to udev_hwdb_new() but I would consider this to be excessively evil.
Comment 1 Allison Lortie (desrt) 2013-12-10 12:22:30 UTC
Created attachment 90565 [details] [review] [PATCH 1/5] udev: drop 'udev' field from udev_list struct This is being used from absolutely nowhere, but is set from udev_list_init(). Drop it.
Comment 2 Allison Lortie (desrt) 2013-12-10 12:22:52 UTC
Created attachment 90566 [details] [review] [PATCH 2/5] udev: drop 'udev' paramter from udev_list_init() Drop the now-unused 'udev' parameter from the internal private API udev_list_init().
Comment 3 Allison Lortie (desrt) 2013-12-10 12:23:14 UTC
Created attachment 90567 [details] [review] [PATCH 3/5] udev: udev_hwdb_new: document unused parameter Rename the 'udev' parameter to 'unused' and document that it is ignored. Also, change its type to void *. We cannot remove the parameter here because this is public API.
Comment 4 Allison Lortie (desrt) 2013-12-10 12:23:38 UTC
Created attachment 90568 [details] [review] [PATCH 4/5] udev: split out new libudev-hwdb.h header Split the udev_hwdb public APIs into a separate libudev-hwdb.h header, including this new header from libudev.h. We also move the udev_list public APIs here because they are needed by users of hwdb. We could add another file just for the list APIs but if we view .h files as public entry points to the API, it makes more sense just to do it as we have done here.
Comment 5 Allison Lortie (desrt) 2013-12-10 12:23:57 UTC
Created attachment 90569 [details] [review] [PATCH 5/5] udev: add libudev-hwdb.pc as copy of libudev.pc Add libudev-hwdb.pc with the same content as libudev.pc. It is now possible for people who only want to use the hwdb functionality of libudev to have 'libudev-hwdb' in their pkg-config and to #include <libudev-hwdb.h> from C. On Linux, the effect of doing this this completely unchanged (since you will still get -ludev) but this will allow other systems to implement the hwdb functionality without having to implement all of udev.
Comment 6 Richard Hughes 2013-12-10 13:01:50 UTC
For reference, something like this is required so freebsd can use colord which uses the hwdb.
Comment 7 Zbigniew Jedrzejewski-Szmek 2013-12-10 13:09:57 UTC
Comment 8 Kay Sievers 2013-12-10 13:58:29 UTC
Na, sorry. HWDB is part of udev and will stay as that. we will not randomly split stuff out.
Comment 9 Zbigniew Jedrzejewski-Szmek 2013-12-10 14:02:39 UTC
(In reply to comment #8) > Na, sorry. HWDB is part of udev and will stay as that. we will not randomly > split stuff out. OK, that's what I thought. What about the first three patches?
Comment 10 Allison Lortie (desrt) 2013-12-10 14:17:26 UTC
fwiw, those first three patches are essentially just common sense...
Comment 11 Kay Sievers 2013-12-10 14:55:03 UTC
Always having a library context allows us to extend things without the need of global/static variables. It's just common sense how things should work today, and not supposed to be optimized away. The rest of libudev also routes the library logging through the context, which allows the library user to receive the log messages directly instead of doing the logging itself. The hwdb code should just use this too.
Comment 12 Bastien Nocera 2014-07-03 14:04:39 UTC
(In reply to comment #8) > Na, sorry. HWDB is part of udev and will stay as that. we will not randomly > split stuff out. Has this changed? There are a few non-Linux, non-systemd places where I'd like to use hwdb to get pnp.ids. See https://bugzilla.gnome.org/show_bug.cgi?id=590059