[Buildroot] [PATCH] optional DBus support in wpa_supplicant

[Buildroot] [PATCH] optional DBus support in wpa_supplicant

[Sven Neumann]

Hi,

the attached patch adds the buildroot config option
BR2_PACKAGE_WPA_SUPPLICANT_DBUS. With this option enabled the DBus
specific files from wpa_supplicant will be installed to the target and
support for the DBus control interface will be compiled into
wpa_supplicant. Please consider to include this patch.


Sven


-------------- next part --------------
A non-text attachment was scrubbed...
Name: wpa-supplicant-dbus.diff
Type: text/x-patch
Size: 8319 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20090713/d57cfec2/attachment.bin>

[Buildroot] [PATCH] optional DBus support in wpa_supplicant

[Sven Neumann]

Hi,

On Mon, 2009-07-13 at 10:20 +0200, Sven Neumann wrote:

> the attached patch adds the buildroot config option
> BR2_PACKAGE_WPA_SUPPLICANT_DBUS. With this option enabled the DBus
> specific files from wpa_supplicant will be installed to the target and
> support for the DBus control interface will be compiled into
> wpa_supplicant. Please consider to include this patch.

After updating our copy of buildroot to the latest snapshot, I found
that the patch does not any longer apply cleanly. Attached is an updated
patch that does.


Sven


-------------- next part --------------
A non-text attachment was scrubbed...
Name: wpa-supplicant-dbus.diff
Type: text/x-patch
Size: 2443 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20090713/3c8321a0/attachment.bin>

[Buildroot] [PATCH] optional DBus support in wpa_supplicant

[Peter Korsgaard]

>>>>> "Sven" == Sven Neumann <s.neumann@raumfeld.com> writes:

 Sven> Hi,
 Sven> On Mon, 2009-07-13 at 10:20 +0200, Sven Neumann wrote:

 >> the attached patch adds the buildroot config option
 >> BR2_PACKAGE_WPA_SUPPLICANT_DBUS. With this option enabled the DBus
 >> specific files from wpa_supplicant will be installed to the target and
 >> support for the DBus control interface will be compiled into
 >> wpa_supplicant. Please consider to include this patch.

 Sven> After updating our copy of buildroot to the latest snapshot, I found
 Sven> that the patch does not any longer apply cleanly. Attached is an updated
 Sven> patch that does.

Thanks, but do we really need a special option for this? Shouldn't
this just always be done if you have dbus enabled?

Similiar to how openssl support is handled in various packages
(E.G. libcurl).

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] optional DBus support in wpa_supplicant

[Sven Neumann]

Hi,

On Mon, 2009-07-13 at 12:44 +0200, Peter Korsgaard wrote:

>  >> the attached patch adds the buildroot config option
>  >> BR2_PACKAGE_WPA_SUPPLICANT_DBUS. With this option enabled the DBus
>  >> specific files from wpa_supplicant will be installed to the target and
>  >> support for the DBus control interface will be compiled into
>  >> wpa_supplicant. Please consider to include this patch.
> 
>  Sven> After updating our copy of buildroot to the latest snapshot, I found
>  Sven> that the patch does not any longer apply cleanly. Attached is an updated
>  Sven> patch that does.
> 
> Thanks, but do we really need a special option for this? Shouldn't
> this just always be done if you have dbus enabled?
> 
> Similiar to how openssl support is handled in various packages
> (E.G. libcurl).

I don't have a strong opinion on that. The DBus control interface adds
about 24kB to the the wpa_supplicant binary size. People might have DBus
enabled for some other reason and don't need this feature in
wpa_supplicant. But then, perhaps it's just not worth the extra
option...


Sven

[Buildroot] [PATCH] optional DBus support in wpa_supplicant

[Gustavo Zacarias]

Sven Neumann wrote:

> I don't have a strong opinion on that. The DBus control interface adds
> about 24kB to the the wpa_supplicant binary size. People might have DBus
> enabled for some other reason and don't need this feature in
> wpa_supplicant. But then, perhaps it's just not worth the extra
> option...
> 
> 
> Sven

Being the one "guilty" of wpa_supplicant updates i don't have a strong
opinion on this either.
On one side wpa_supplicant has it's own way of controlling itself via
wpa_cli and thus somewhat negates the need for dbus bindings.
On the other side you may not want to fork to call wpa_cli and/or learn
the wpa_supplicant/cli controlling protocol (or other good reasons), and
if you already have dbus enabled those extra 24k probably won't hurt
that much in the global scheme of things - after all people who want to
save that last KB will probably steer away from dbus.
Regards.

[Buildroot] [PATCH] optional DBus support in wpa_supplicant

[Peter Korsgaard]

>>>>> "Gustavo" == Gustavo Zacarias <gustavo@zacarias.com.ar> writes:

Hi,

 Gustavo> Being the one "guilty" of wpa_supplicant updates i don't
 Gustavo> have a strong opinion on this either.  On one side
 Gustavo> wpa_supplicant has it's own way of controlling itself via
 Gustavo> wpa_cli and thus somewhat negates the need for dbus
 Gustavo> bindings.  On the other side you may not want to fork to
 Gustavo> call wpa_cli and/or learn the wpa_supplicant/cli controlling
 Gustavo> protocol (or other good reasons), and if you already have
 Gustavo> dbus enabled those extra 24k probably won't hurt that much
 Gustavo> in the global scheme of things - after all people who want
 Gustavo> to save that last KB will probably steer away from dbus.

Indeed. For big dependencies like dbus (or openssl), then I would
prefer BR to just do the right thing (tm) instead of adding very
detailed configuration options for things that only increase the size
with a few %.

In the case of dbus, the minimal configuration (including
dependencies) is around ~1MB, so I don't think it's worthwhile
discussing about 24kb overhead.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] optional DBus support in wpa_supplicant

[Sven Neumann]

Hi,

attached is an updated patch that enables the DBus config interface in
wpa-supplicant if DBus is enabled in buildroot without introducing
another config option.


Sven


-------------- next part --------------
A non-text attachment was scrubbed...
Name: wpa-supplicant-dbus.diff
Type: text/x-patch
Size: 1782 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20090714/affec8b1/attachment-0001.bin>

[Buildroot] [PATCH] optional DBus support in wpa_supplicant

[Peter Korsgaard]

>>>>> "Sven" == Sven Neumann <s.neumann@raumfeld.com> writes:

 Sven> Hi,
 Sven> attached is an updated patch that enables the DBus config interface in
 Sven> wpa-supplicant if DBus is enabled in buildroot without introducing
 Sven> another config option.

Thanks!

 Sven> --- ../buildroot-orig/package/wpa_supplicant/wpa_supplicant.mk	2009-07-12 21:13:04.000000000 +0200
 Sven> +++ package/wpa_supplicant/wpa_supplicant.mk	2009-07-13 11:07:52.000000000 +0200
 Sven> @@ -13,10 +13,14 @@
 Sven>  WPA_SUPPLICANT_CONFIG = $(WPA_SUPPLICANT_DIR)/wpa_supplicant/.config
 Sven>  WPA_SUPPLICANT_SUBDIR = wpa_supplicant
 Sven>  WPA_SUPPLICANT_TARGET_BINS = wpa_cli wpa_supplicant wpa_passphrase
 Sven> +WPA_SUPPLICANT_DBUS_SERVICE = fi.epitest.hostap.WPASupplicant

Aren't you missing a ".service" here? Service files seems to normally
have the .service extension, and the wpa supplicant file on my machine
(Debian) does as well.

 Sven> +	$(INSTALL) -D \
 Sven> +	  $(WPA_SUPPLICANT_DIR)/wpa_supplicant/dbus-wpa_supplicant.service \
 Sven> +	  $(TARGET_DIR)/usr/share/dbus-1/system-services/$(WPA_SUPPLICANT_DBUS_SERVICE)

Here you don't add it
 
 Sven> +	rm -f $(TARGET_DIR)/usr/share/dbus-1/system-services/$(WPA_SUPPLICANT_DBUS_SERVICE).service

But here you do.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] optional DBus support in wpa_supplicant

[Sven Neumann]

Hi,

On Tue, 2009-07-14 at 21:20 +0200, Peter Korsgaard wrote:

>  Sven> --- ../buildroot-orig/package/wpa_supplicant/wpa_supplicant.mk	2009-07-12 21:13:04.000000000 +0200
>  Sven> +++ package/wpa_supplicant/wpa_supplicant.mk	2009-07-13 11:07:52.000000000 +0200
>  Sven> @@ -13,10 +13,14 @@
>  Sven>  WPA_SUPPLICANT_CONFIG = $(WPA_SUPPLICANT_DIR)/wpa_supplicant/.config
>  Sven>  WPA_SUPPLICANT_SUBDIR = wpa_supplicant
>  Sven>  WPA_SUPPLICANT_TARGET_BINS = wpa_cli wpa_supplicant wpa_passphrase
>  Sven> +WPA_SUPPLICANT_DBUS_SERVICE = fi.epitest.hostap.WPASupplicant
> 
> Aren't you missing a ".service" here? Service files seems to normally
> have the .service extension, and the wpa supplicant file on my machine
> (Debian) does as well.
> 
>  Sven> +	$(INSTALL) -D \
>  Sven> +	  $(WPA_SUPPLICANT_DIR)/wpa_supplicant/dbus-wpa_supplicant.service \
>  Sven> +	  $(TARGET_DIR)/usr/share/dbus-1/system-services/$(WPA_SUPPLICANT_DBUS_SERVICE)
> 
> Here you don't add it
>  
>  Sven> +	rm -f $(TARGET_DIR)/usr/share/dbus-1/system-services/$(WPA_SUPPLICANT_DBUS_SERVICE).service
> 
> But here you do.

Yes, the suffix is missing there. I didn't actually use the DBus
activation feature that relies on this file. So I didn't notice that I
got the filename wrong. Well spotted.


Sven

[Buildroot] [PATCH] optional DBus support in wpa_supplicant

[Sven Neumann]

Hi Peter,

attached is an updated patch that has the missing file suffix fixed that
you spotted in my last patch.


Sven


-------------- next part --------------
A non-text attachment was scrubbed...
Name: wpa-supplicant-dbus.diff
Type: text/x-patch
Size: 1790 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20090720/2ad49064/attachment.bin>

[Buildroot] [PATCH] optional DBus support in wpa_supplicant

[Peter Korsgaard]

>>>>> "Sven" == Sven Neumann <s.neumann@raumfeld.com> writes:

 Sven> Hi Peter,

 Sven> attached is an updated patch that has the missing file suffix
 Sven> fixed that you spotted in my last patch.

Thanks, committed.

-- 
Bye, Peter Korsgaard