| Message ID | 20170825142120.26160-1-aleksander@aleksander.es |
|---|---|
| State | Changes Requested |
| Headers | show |
Hello, On Fri, 25 Aug 2017 16:21:20 +0200, Aleksander Morgado wrote: > The dbus-glib package isn't a dependency since ModemManager 1.0, which > is based on libglib2's GDBus implementation. > > Also, explicitly set libglib2 as dependency, which currently was being > implicitly included by libgudev. The next major ModemManager release > will have udev/libgudev as optional packages, while libglib2 is > definitely not going to be ever optional. > > Finally, also set dbus as a dependency, as ModemManager won't work > without a system DBus available. I was about to apply this, but this last part seems wrong: if DBus is only needed at runtime (and not build time), then having the select BR2_PACKAGE_DBUS in Config.in is enough: it ensures DBus will be built, it's just that you don't have the guarantee it will be built before modem-manager. So: - If DBus is only a runtime dependency, don't add dbus do MODEM_MANAGER_DEPENDENCIES, and instead add a comment above select BR2_PACKAGE_DBUS that says "# runtime dependency" - If DBus is a build-time dependency, then your change is correct, but your commit log is unclear. Thanks! Thomas
> On Fri, 25 Aug 2017 16:21:20 +0200, Aleksander Morgado wrote: >> The dbus-glib package isn't a dependency since ModemManager 1.0, which >> is based on libglib2's GDBus implementation. >> >> Also, explicitly set libglib2 as dependency, which currently was being >> implicitly included by libgudev. The next major ModemManager release >> will have udev/libgudev as optional packages, while libglib2 is >> definitely not going to be ever optional. >> >> Finally, also set dbus as a dependency, as ModemManager won't work >> without a system DBus available. > > I was about to apply this, but this last part seems wrong: if DBus is > only needed at runtime (and not build time), then having the select > BR2_PACKAGE_DBUS in Config.in is enough: it ensures DBus will be built, > it's just that you don't have the guarantee it will be built before > modem-manager. > > So: > > - If DBus is only a runtime dependency, don't add dbus do > MODEM_MANAGER_DEPENDENCIES, and instead add a comment above select > BR2_PACKAGE_DBUS that says "# runtime dependency" > > - If DBus is a build-time dependency, then your change is correct, but > your commit log is unclear. > Understood; yes DBus is only runtime dependency, so the Config.in change would be enough. I guess the same happens with udev then? MM doesn't build-depend on udev, only on gudev (although it seems that gudev itself depends on udev).
Hello, On Sun, 27 Aug 2017 09:05:15 +0200, Aleksander Morgado wrote: > Understood; yes DBus is only runtime dependency, so the Config.in > change would be enough. Well, the package already has "select BR2_PACKAGE_DBUS", so that shouldn't require any change. > I guess the same happens with udev then? MM > doesn't build-depend on udev, only on gudev (although it seems that > gudev itself depends on udev). Then yes, having "udev" in MODEM_MANAGER_DEPENDENCIES is useless. Best regards, Thomas
>> Understood; yes DBus is only runtime dependency, so the Config.in >> change would be enough. > > Well, the package already has "select BR2_PACKAGE_DBUS", so that > shouldn't require any change. > Yeah, I meant the (already existing) Config.in entry would be enough, no change expected there, sorry for the confusion. >> I guess the same happens with udev then? MM >> doesn't build-depend on udev, only on gudev (although it seems that >> gudev itself depends on udev). > > Then yes, having "udev" in MODEM_MANAGER_DEPENDENCIES is useless. > Will update the patch and resend, thanks for the clarifications.
diff --git a/package/modem-manager/Config.in b/package/modem-manager/Config.in index aa7ed1e2f..3b32acba3 100644 --- a/package/modem-manager/Config.in +++ b/package/modem-manager/Config.in @@ -5,7 +5,7 @@ config BR2_PACKAGE_MODEM_MANAGER depends on BR2_TOOLCHAIN_HAS_THREADS # dbus, libglib2 depends on BR2_USE_MMU # dbus select BR2_PACKAGE_DBUS - select BR2_PACKAGE_DBUS_GLIB + select BR2_PACKAGE_LIBGLIB2 select BR2_PACKAGE_LIBGUDEV help ModemManager is a DBus-activated daemon which controls mobile diff --git a/package/modem-manager/modem-manager.mk b/package/modem-manager/modem-manager.mk index 6459f8785..ce6d5dfc1 100644 --- a/package/modem-manager/modem-manager.mk +++ b/package/modem-manager/modem-manager.mk @@ -9,7 +9,7 @@ MODEM_MANAGER_SOURCE = ModemManager-$(MODEM_MANAGER_VERSION).tar.xz MODEM_MANAGER_SITE = http://www.freedesktop.org/software/ModemManager MODEM_MANAGER_LICENSE = GPL-2.0+ (programs, plugins), LGPL-2.0+ (libmm-glib) MODEM_MANAGER_LICENSE_FILES = COPYING -MODEM_MANAGER_DEPENDENCIES = host-pkgconf udev dbus-glib host-intltool libgudev +MODEM_MANAGER_DEPENDENCIES = host-pkgconf host-intltool dbus udev libglib2 libgudev MODEM_MANAGER_INSTALL_STAGING = YES ifeq ($(BR2_PACKAGE_MODEM_MANAGER_LIBQMI),y)
The dbus-glib package isn't a dependency since ModemManager 1.0, which is based on libglib2's GDBus implementation. Also, explicitly set libglib2 as dependency, which currently was being implicitly included by libgudev. The next major ModemManager release will have udev/libgudev as optional packages, while libglib2 is definitely not going to be ever optional. Finally, also set dbus as a dependency, as ModemManager won't work without a system DBus available. Signed-off-by: Aleksander Morgado <aleksander@aleksander.es> --- package/modem-manager/Config.in | 2 +- package/modem-manager/modem-manager.mk | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.14.1