Patchwork build: allow overriding D-Bus version from pkg-config

login
register
mail settings
Submitter Grant Erickson
Date Jan. 4, 2012, 10:37 p.m.
Message ID <1325716671-25307-1-git-send-email-marathon96@gmail.com>
Download mbox | patch
Permalink /patch/134374/
State Superseded
Headers show

Comments

Grant Erickson - Jan. 4, 2012, 10:37 p.m.
This allows the package build to specify the D-Bus version being built
against externally to pkg-config.

Signed-off-by: Grant Erickson <marathon96@gmail.com>
---
 wpa_supplicant/Makefile |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)
Dan Williams - Jan. 4, 2012, 11:12 p.m.
On Wed, 2012-01-04 at 14:37 -0800, Grant Erickson wrote:
> This allows the package build to specify the D-Bus version being built
> against externally to pkg-config.

What does this actually do?  I don't see DBUS_VERSION actually used
anywhere in the wpa_supplicant code or makefiles, nor is it used in the
D-Bus headers.  Perhaps we should just remove those pieces?

Dan

> Signed-off-by: Grant Erickson <marathon96@gmail.com>
> ---
>  wpa_supplicant/Makefile |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/wpa_supplicant/Makefile b/wpa_supplicant/Makefile
> index 0832f10..554c22a 100644
> --- a/wpa_supplicant/Makefile
> +++ b/wpa_supplicant/Makefile
> @@ -1161,7 +1161,11 @@ endif
>  ifndef DBUS_INCLUDE
>  DBUS_INCLUDE := $(shell $(PKG_CONFIG) --cflags dbus-1)
>  endif
> -dbus_version=$(subst ., ,$(shell $(PKG_CONFIG) --modversion dbus-1))
> +ifndef DBUS_VERSION
> +dbus_version := $(subst ., ,$(shell $(PKG_CONFIG) --modversion dbus-1))
> +else
> +dbus_version := $(subst ., ,$(DBUS_VERSION))
> +endif
>  DBUS_VERSION_MAJOR=$(word 1,$(dbus_version))
>  DBUS_VERSION_MINOR=$(word 2,$(dbus_version))
>  ifeq ($(DBUS_VERSION_MAJOR),)
Grant Erickson - Jan. 5, 2012, 7:15 a.m.
On Wed, Jan 4, 2012 at 3:12 PM, Dan Williams <dcbw@redhat.com> wrote:
> On Wed, 2012-01-04 at 14:37 -0800, Grant Erickson wrote:
>> This allows the package build to specify the D-Bus version being built
>> against externally to pkg-config.
>
> What does this actually do?  I don't see DBUS_VERSION actually used
> anywhere in the wpa_supplicant code or makefiles, nor is it used in the
> D-Bus headers.  Perhaps we should just remove those pieces?

Dan,

I've no idea what the intent originally was or what it is supposed to do
today.

Searching through existing packages in my current project indicates that
avahi is doing something similar and has some conditional code based on the
extracted major and minor values:

% grep -ri 'DBUS_VERSION\(_MAJOR\|_MINOR\)' avahi/ connman/ dbus/ hostap/
libnih/ upstart/ | files -su
avahi/avahi-0.6.28/avahi-common/dbus-watch-glue.c
avahi/avahi-0.6.28/avahi-daemon/dbus-protocol.c
avahi/avahi-0.6.28/configure
avahi/avahi-0.6.28/configure.ac
hostap/hostap/wpa_supplicant/Android.mk
hostap/hostap/wpa_supplicant/dbus/Makefile
hostap/hostap/wpa_supplicant/Makefile

However, beyond the cited make files, hostap does nothing with them nor
does D-Bus itself.

That said, I am OK with getting rid of the checks altogether.

-Grant
Jouni Malinen - Jan. 9, 2012, 4:15 a.m.
On Wed, Jan 04, 2012 at 11:15:18PM -0800, Grant Erickson wrote:
> Searching through existing packages in my current project indicates that
> avahi is doing something similar and has some conditional code based on the
> extracted major and minor values:
> 
> % grep -ri 'DBUS_VERSION\(_MAJOR\|_MINOR\)' avahi/ connman/ dbus/ hostap/

> However, beyond the cited make files, hostap does nothing with them nor
> does D-Bus itself.
> 
> That said, I am OK with getting rid of the checks altogether.

If no one can identify clear use for these, I would prefer to get rid of
them rather than extending the parameters for some hypothetical use.
Kel Modderman - Jan. 12, 2012, 9:01 p.m.
> On Wed, Jan 04, 2012 at 11:15:18PM -0800, Grant Erickson wrote:
> > Searching through existing packages in my current project indicates that
> > avahi is doing something similar and has some conditional code based on the
> > extracted major and minor values:
> > 
> > % grep -ri 'DBUS_VERSION\(_MAJOR\|_MINOR\)' avahi/ connman/ dbus/ hostap/
> 
> > However, beyond the cited make files, hostap does nothing with them nor
> > does D-Bus itself.
> > 
> > That said, I am OK with getting rid of the checks altogether.
> 
> If no one can identify clear use for these, I would prefer to get rid of
> them rather than extending the parameters for some hypothetical use.
> 
> 

Was this why they were added?

http://lists.shmoo.com/pipermail/hostap/2007-November/016463.html

Maybe they're not needed today.

Thanks, Kel.
Jouni Malinen - Jan. 29, 2012, 10:48 a.m.
On Fri, Jan 13, 2012 at 07:01:44AM +1000, Kel Modderman wrote:
> Was this why they were added?
> 
> http://lists.shmoo.com/pipermail/hostap/2007-November/016463.html

Thanks! Indeed, that seems to be source of the version definitions.

> Maybe they're not needed today.

It looks like the new D-Bus interface implementation did not have the
backwards compatibility code and commit
8ddef94bd41747ba658ed4ed5dfa9e62b4b84cfa combined the initialization
code in a way that the old interface ended up using the new code. At
that point, the compatibility wrapper for dbus_watch_get_unix_fd()
disappeared. Since no one has complained about this over the last two
years, it looks like no one cares about the old version anymore. I'll
remove the now unused version definitions.

Patch

diff --git a/wpa_supplicant/Makefile b/wpa_supplicant/Makefile
index 0832f10..554c22a 100644
--- a/wpa_supplicant/Makefile
+++ b/wpa_supplicant/Makefile
@@ -1161,7 +1161,11 @@  endif
 ifndef DBUS_INCLUDE
 DBUS_INCLUDE := $(shell $(PKG_CONFIG) --cflags dbus-1)
 endif
-dbus_version=$(subst ., ,$(shell $(PKG_CONFIG) --modversion dbus-1))
+ifndef DBUS_VERSION
+dbus_version := $(subst ., ,$(shell $(PKG_CONFIG) --modversion dbus-1))
+else
+dbus_version := $(subst ., ,$(DBUS_VERSION))
+endif
 DBUS_VERSION_MAJOR=$(word 1,$(dbus_version))
 DBUS_VERSION_MINOR=$(word 2,$(dbus_version))
 ifeq ($(DBUS_VERSION_MAJOR),)