diff mbox

[1/1] package/network-manager: NM needs ISC-DHCPC or DHCPCD

Message ID 1416983821-2729-1-git-send-email-nroach44@gmail.com
State Superseded
Headers show

Commit Message

Nathaniel Roach Nov. 26, 2014, 6:37 a.m. UTC
As NM communicates with the dhcp client through DBus, it
doesn't support any old client.

Add the dependencies to Config.in, and ensure that they
are built beforehand.
---
Two things I'm a little wary of:
 - The new "you need to enable x,y,z..." line is /huge/,
	is there a nice way around this?
 - This adds unecessary things to static-ip configured
	systems, but then again they are using NM for that
	(instead of ifup/down) so maybe this is a non issue?

Signed-off-by: Nathaniel Roach <nroach44@gmail.com>
---
 package/network-manager/Config.in          | 6 ++++--
 package/network-manager/network-manager.mk | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

Comments

Baruch Siach Nov. 26, 2014, 6:45 a.m. UTC | #1
Hi Nathaniel,

On Wed, Nov 26, 2014 at 02:37:01PM +0800, Nathaniel Roach wrote:
> As NM communicates with the dhcp client through DBus, it
> doesn't support any old client.
> 
> Add the dependencies to Config.in, and ensure that they
> are built beforehand.
> ---
> Two things I'm a little wary of:
>  - The new "you need to enable x,y,z..." line is /huge/,
> 	is there a nice way around this?
>  - This adds unecessary things to static-ip configured
> 	systems, but then again they are using NM for that
> 	(instead of ifup/down) so maybe this is a non issue?
> 
> Signed-off-by: Nathaniel Roach <nroach44@gmail.com>
> ---
>  package/network-manager/Config.in          | 6 ++++--
>  package/network-manager/network-manager.mk | 2 ++
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/package/network-manager/Config.in b/package/network-manager/Config.in
> index 0ed6ca5..72a2732 100644
> --- a/package/network-manager/Config.in
> +++ b/package/network-manager/Config.in
> @@ -8,6 +8,7 @@ config BR2_PACKAGE_NETWORK_MANAGER
>  	depends on BR2_USE_MMU # dbus
>  	depends on BR2_PACKAGE_HAS_UDEV
>  	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_7
> +	depends on BR2_PACKAGE_DHCP_CLIENT || BR2_PACKAGE_DHCPCD

We usually use 'select' for this kind of dependencies. Something like 
(untested):

	select BR2_PACKAGE_DHCPCD if !BR2_PACKAGE_DHCP_CLIENT

baruch
Nathaniel Roach Nov. 26, 2014, 6:55 a.m. UTC | #2
Hi Baruch,

On 26/11/14 14:45, Baruch Siach wrote:
> Hi Nathaniel,
> 
> On Wed, Nov 26, 2014 at 02:37:01PM +0800, Nathaniel Roach wrote:
>>  	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_7
>> +	depends on BR2_PACKAGE_DHCP_CLIENT || BR2_PACKAGE_DHCPCD
> 
> We usually use 'select' for this kind of dependencies. Something like 
> (untested):
> 
> 	select BR2_PACKAGE_DHCPCD if !BR2_PACKAGE_DHCP_CLIENT
> 
> baruch
> 

Ah, okay. I was wondering about that, thanks.

I'll leave this version up for now as my other questions are still
valid. Furthermore, is there a preference as to DHCPCD vs ISC?

Thanks, Nathaniel.
Baruch Siach Nov. 26, 2014, 6:57 a.m. UTC | #3
Hi Nathaniel,

On Wed, Nov 26, 2014 at 02:55:14PM +0800, Nathaniel Roach wrote:
> On 26/11/14 14:45, Baruch Siach wrote:
> > On Wed, Nov 26, 2014 at 02:37:01PM +0800, Nathaniel Roach wrote:
> >>  	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_7
> >> +	depends on BR2_PACKAGE_DHCP_CLIENT || BR2_PACKAGE_DHCPCD
> > 
> > We usually use 'select' for this kind of dependencies. Something like 
> > (untested):
> > 
> > 	select BR2_PACKAGE_DHCPCD if !BR2_PACKAGE_DHCP_CLIENT
> 
> Ah, okay. I was wondering about that, thanks.
> 
> I'll leave this version up for now as my other questions are still
> valid. Furthermore, is there a preference as to DHCPCD vs ISC?

I think we should default to the more lightweight one. I guess it's dhcpcd in 
this case.

baruch
Nathaniel Roach Nov. 26, 2014, 7:07 a.m. UTC | #4
Hi Baruch,

On 26/11/14 14:57, Baruch Siach wrote:> Hi Nathaniel,
>
> On Wed, Nov 26, 2014 at 02:55:14PM +0800, Nathaniel Roach wrote:
>> On 26/11/14 14:45, Baruch Siach wrote:
>>> On Wed, Nov 26, 2014 at 02:37:01PM +0800, Nathaniel Roach wrote:
>>>>  	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_7
>>>> +	depends on BR2_PACKAGE_DHCP_CLIENT || BR2_PACKAGE_DHCPCD
>>>
>>> We usually use 'select' for this kind of dependencies. Something like
>>> (untested):
>>>
>>> 	select BR2_PACKAGE_DHCPCD if !BR2_PACKAGE_DHCP_CLIENT
>>
>> Ah, okay. I was wondering about that, thanks.
>>
>> I'll leave this version up for now as my other questions are still
>> valid. Furthermore, is there a preference as to DHCPCD vs ISC?
>
> I think we should default to the more lightweight one. I guess it's
dhcpcd in
> this case.
>
> baruch
>

Cheers. The line you wrote worked exactly as is, and as I've just
realised that my first question is no longer valid, I'll update the patch.

Nathaniel.
Gustavo Zacarias Nov. 26, 2014, 10:06 a.m. UTC | #5
On 11/26/2014 03:57 AM, Baruch Siach wrote:

>>> We usually use 'select' for this kind of dependencies. Something like 
>>> (untested):
>>>
>>> 	select BR2_PACKAGE_DHCPCD if !BR2_PACKAGE_DHCP_CLIENT
>>
>> Ah, okay. I was wondering about that, thanks.
>>
>> I'll leave this version up for now as my other questions are still
>> valid. Furthermore, is there a preference as to DHCPCD vs ISC?
> 
> I think we should default to the more lightweight one. I guess it's dhcpcd in 
> this case.

+1, the ISC DHCP 4.1.x series even though ESV (extended support) will
EOL (end-of-life) by december, and even though i don't know which one
will be the new ESV (likely 4.3.x) 4.2.x+ bundles bind libraries, so
ouch for bloat.
Regards.
diff mbox

Patch

diff --git a/package/network-manager/Config.in b/package/network-manager/Config.in
index 0ed6ca5..72a2732 100644
--- a/package/network-manager/Config.in
+++ b/package/network-manager/Config.in
@@ -8,6 +8,7 @@  config BR2_PACKAGE_NETWORK_MANAGER
 	depends on BR2_USE_MMU # dbus
 	depends on BR2_PACKAGE_HAS_UDEV
 	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_7
+	depends on BR2_PACKAGE_DHCP_CLIENT || BR2_PACKAGE_DHCPCD
 	select BR2_PACKAGE_DBUS
 	select BR2_PACKAGE_DBUS_GLIB
 	select BR2_PACKAGE_GNUTLS
@@ -52,9 +53,10 @@  config BR2_PACKAGE_NETWORK_MANAGER_PPPD
 	  This option enables support for PPPD daemon
 endif
 
-comment "NetworkManager needs udev /dev management and a toolchain w/ IPv6, largefile, wchar, threads, headers >= 3.7"
+comment "NetworkManager needs udev /dev management and a toolchain w/ IPv6, largefile, wchar, threads, headers >= 3.7 and ISC DHCP client or DHCPCD"
 	depends on !BR2_avr32
 	depends on BR2_USE_MMU
 	depends on !BR2_INET_IPV6 || !BR2_LARGEFILE || !BR2_USE_WCHAR || \
 		!BR2_TOOLCHAIN_HAS_THREADS || \
-		!BR2_PACKAGE_HAS_UDEV || !BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_7
+		!BR2_PACKAGE_HAS_UDEV || !BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_7 || \
+		!(BR2_PACKAGE_DHCP_CLIENT || BR2_PACKAGE_DHCPCD)
diff --git a/package/network-manager/network-manager.mk b/package/network-manager/network-manager.mk
index 7944254..96bc858 100644
--- a/package/network-manager/network-manager.mk
+++ b/package/network-manager/network-manager.mk
@@ -57,10 +57,12 @@  endif
 
 ifeq ($(BR2_PACKAGE_DHCP_CLIENT),y)
 NETWORK_MANAGER_CONF_OPTS += --with-dhclient=/usr/sbin/dhclient
+NETWORK_MANAGER_DEPENDENCIES += dhcp
 endif
 
 ifeq ($(BR2_PACKAGE_DHCPCD),y)
 NETWORK_MANAGER_CONF_OPTS += --with-dhcpcd=/usr/sbin/dhcpcd
+NETWORK_MANAGER_DEPENDENCIES += dhcpcd
 endif
 
 # uClibc by default doesn't have backtrace support, so don't use it