[Buildroot] [PATCH 1/1] package/network-manager: add optional nmcli support

[Buildroot] [PATCH 1/1] package/network-manager: add optional nmcli support

[Michael Fischer]

When NetworkManager is built without the READLINE package the nmcli was not build.

Signed-off-by: Michael Fischer <mf@go-sys.de>
---
 package/network-manager/Config.in          | 7 ++++++-
 package/network-manager/network-manager.mk | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/package/network-manager/Config.in b/package/network-manager/Config.in
index a48cb37b15..1a123a26e2 100644
--- a/package/network-manager/Config.in
+++ b/package/network-manager/Config.in
@@ -37,6 +37,12 @@ config BR2_PACKAGE_NETWORK_MANAGER_TUI
 	help
 	  This option enables terminal based UI
 
+config BR2_PACKAGE_NETWORK_MANAGER_CLI
+        bool "nmcli support"
+        select BR2_PACKAGE_READLINE
+        help
+          This option enables support for NetworkManager Command Line Interface
+
 config BR2_PACKAGE_NETWORK_MANAGER_MODEM_MANAGER
 	bool "modem-manager support"
 	select BR2_PACKAGE_MODEM_MANAGER
@@ -54,7 +60,6 @@ config BR2_PACKAGE_NETWORK_MANAGER_OVS
 	select BR2_PACKAGE_JANSSON
 	help
 	  This option enables support for OpenVSwitch
-
 endif
 
 comment "NetworkManager needs udev /dev management and a glibc toolchain w/ headers >= 4.6, dynamic library, wchar, threads, gcc >= 4.9"
diff --git a/package/network-manager/network-manager.mk b/package/network-manager/network-manager.mk
index 974320fce0..1eb9233b44 100644
--- a/package/network-manager/network-manager.mk
+++ b/package/network-manager/network-manager.mk
@@ -135,7 +135,7 @@ else
 NETWORK_MANAGER_CONF_OPTS += --disable-polkit
 endif
 
-ifeq ($(BR2_PACKAGE_READLINE),y)
+ifeq ($(BR2_PACKAGE_NETWORK_MANAGER_CLI),y)
 NETWORK_MANAGER_DEPENDENCIES += readline
 NETWORK_MANAGER_CONF_OPTS += --with-nmcli
 else
-- 
2.20.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/network-manager: add optional nmcli support

[Thomas Petazzoni]

Hello,

+Yann on legacy handling (see below)

On Wed, 26 Jan 2022 10:04:57 +0100
Michael Fischer <mf@go-sys.de> wrote:

> When NetworkManager is built without the READLINE package the nmcli was not build.

Well, this was kind of expected, and isn't really a correct
explanation for this change.

A better explanation I believe is more something like this:

"""
The network-manager package builds the nmcli utility when the readline
package is enabled. However, this is not necessarily obvious to the
user. Therefore, this commit adds an explicit option to enable the
nmcli tool, which automatically selects readline.
"""

> 
> Signed-off-by: Michael Fischer <mf@go-sys.de>

> +config BR2_PACKAGE_NETWORK_MANAGER_CLI
> +        bool "nmcli support"
> +        select BR2_PACKAGE_READLINE
> +        help
> +          This option enables support for NetworkManager Command Line Interface

I think this line is too long, make sure to run "make check-package".

But a bigger problem is the legacy handling. Indeed, before your patch,
a configuration with BR2_PACKAGE_NETWORK_MANAGER=y and
BR2_PACKAGE_READLINE=y gets nmcli. After your patch, such a
configuration no longer has nmcli compiled, because
BR2_PACKAGE_NETWORK_MANAGER_CLI is not enabled.

Is this important to address, I don't know.

Is it possible to address is by doing:

config BR2_PACKAGE_NETWORK_MANAGER_CLI
        bool "nmcli support"
	default y if BR2_PACKAGE_READLINE
        select BR2_PACKAGE_READLINE

I don't know if Kconfig is happy about such a construct. Yann? :-)

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/network-manager: add optional nmcli support

[Yann E. MORIN]

Thomas, Michael, All,

On 2022-01-26 23:14 +0100, Thomas Petazzoni spake thusly:
> On Wed, 26 Jan 2022 10:04:57 +0100
> Michael Fischer <mf@go-sys.de> wrote:
[--SNIP--]
> > +config BR2_PACKAGE_NETWORK_MANAGER_CLI
> > +        bool "nmcli support"
> > +        select BR2_PACKAGE_READLINE
> > +        help
> > +          This option enables support for NetworkManager Command Line Interface
> 
> I think this line is too long, make sure to run "make check-package".
> 
> But a bigger problem is the legacy handling. Indeed, before your patch,
> a configuration with BR2_PACKAGE_NETWORK_MANAGER=y and
> BR2_PACKAGE_READLINE=y gets nmcli. After your patch, such a
> configuration no longer has nmcli compiled, because
> BR2_PACKAGE_NETWORK_MANAGER_CLI is not enabled.

I was also looking at that patch, and was wondering what we could do.
I do not like efaulting symbols to 'y', even conditionally, and I would
also expect people to regression-test their systems when they upgrade
anyway.

> Is this important to address, I don't know.
> 
> Is it possible to address is by doing:
> 
> config BR2_PACKAGE_NETWORK_MANAGER_CLI
> 	 bool "nmcli support"
> 	 default y if BR2_PACKAGE_READLINE
> 	 select BR2_PACKAGE_READLINE
> 
> I don't know if Kconfig is happy about such a construct. Yann? :-)

No it is not valid: you have both a backward and a forward dependency
that drive the availability of the symbol:

    package/readline/Config.in:1:error: recursive dependency detected!
    package/readline/Config.in:1:   symbol BR2_PACKAGE_READLINE is selected by BR2_PACKAGE_NETWORK_MANAGER_CLI
    package/network-manager/Config.in:36:   symbol BR2_PACKAGE_NETWORK_MANAGER_CLI depends on BR2_PACKAGE_READLINE
    For a resolution refer to Documentation/kbuild/kconfig-language.txt
    subsection "Kconfig recursive dependency limitations"

TBH, I would just leave it like that...

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2 1/1] package/network-manager: add optional nmcli support

[Michael Fischer]

The network-manager package builds the nmcli utility when the
readline package is enabled. However, this is not necessarily
obvious to the user. Therefore, this commit adds an explicit
option to enable the nmcli tool, which automatically selects readline.

Signed-off-by: Michael Fischer <mf@go-sys.de>
---
 package/network-manager/Config.in          | 8 +++++++-
 package/network-manager/network-manager.mk | 2 +-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/package/network-manager/Config.in b/package/network-manager/Config.in
index a48cb37b15..f61a046a95 100644
--- a/package/network-manager/Config.in
+++ b/package/network-manager/Config.in
@@ -37,6 +37,13 @@ config BR2_PACKAGE_NETWORK_MANAGER_TUI
 	help
 	  This option enables terminal based UI
 
+config BR2_PACKAGE_NETWORK_MANAGER_CLI
+	bool "nmcli support"
+	select BR2_PACKAGE_READLINE
+	help
+	  This option enables support for the
+	  NetworkManager Command Line Interface
+
 config BR2_PACKAGE_NETWORK_MANAGER_MODEM_MANAGER
 	bool "modem-manager support"
 	select BR2_PACKAGE_MODEM_MANAGER
@@ -54,7 +61,6 @@ config BR2_PACKAGE_NETWORK_MANAGER_OVS
 	select BR2_PACKAGE_JANSSON
 	help
 	  This option enables support for OpenVSwitch
-
 endif
 
 comment "NetworkManager needs udev /dev management and a glibc toolchain w/ headers >= 4.6, dynamic library, wchar, threads, gcc >= 4.9"
diff --git a/package/network-manager/network-manager.mk b/package/network-manager/network-manager.mk
index 974320fce0..1eb9233b44 100644
--- a/package/network-manager/network-manager.mk
+++ b/package/network-manager/network-manager.mk
@@ -135,7 +135,7 @@ else
 NETWORK_MANAGER_CONF_OPTS += --disable-polkit
 endif
 
-ifeq ($(BR2_PACKAGE_READLINE),y)
+ifeq ($(BR2_PACKAGE_NETWORK_MANAGER_CLI),y)
 NETWORK_MANAGER_DEPENDENCIES += readline
 NETWORK_MANAGER_CONF_OPTS += --with-nmcli
 else
-- 
2.20.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 1/1] package/network-manager: add optional nmcli support

[Yann E. MORIN]

Michael, All,

On 2022-01-27 11:46 +0100, Michael Fischer spake thusly:
> The network-manager package builds the nmcli utility when the
> readline package is enabled. However, this is not necessarily
> obvious to the user. Therefore, this commit adds an explicit
> option to enable the nmcli tool, which automatically selects readline.
> 
> Signed-off-by: Michael Fischer <mf@go-sys.de>

Applied to master^Wnext, thanks.

Regards,
Yann E. MORIN.

> ---
>  package/network-manager/Config.in          | 8 +++++++-
>  package/network-manager/network-manager.mk | 2 +-
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/package/network-manager/Config.in b/package/network-manager/Config.in
> index a48cb37b15..f61a046a95 100644
> --- a/package/network-manager/Config.in
> +++ b/package/network-manager/Config.in
> @@ -37,6 +37,13 @@ config BR2_PACKAGE_NETWORK_MANAGER_TUI
>  	help
>  	  This option enables terminal based UI
>  
> +config BR2_PACKAGE_NETWORK_MANAGER_CLI
> +	bool "nmcli support"
> +	select BR2_PACKAGE_READLINE
> +	help
> +	  This option enables support for the
> +	  NetworkManager Command Line Interface
> +
>  config BR2_PACKAGE_NETWORK_MANAGER_MODEM_MANAGER
>  	bool "modem-manager support"
>  	select BR2_PACKAGE_MODEM_MANAGER
> @@ -54,7 +61,6 @@ config BR2_PACKAGE_NETWORK_MANAGER_OVS
>  	select BR2_PACKAGE_JANSSON
>  	help
>  	  This option enables support for OpenVSwitch
> -
>  endif
>  
>  comment "NetworkManager needs udev /dev management and a glibc toolchain w/ headers >= 4.6, dynamic library, wchar, threads, gcc >= 4.9"
> diff --git a/package/network-manager/network-manager.mk b/package/network-manager/network-manager.mk
> index 974320fce0..1eb9233b44 100644
> --- a/package/network-manager/network-manager.mk
> +++ b/package/network-manager/network-manager.mk
> @@ -135,7 +135,7 @@ else
>  NETWORK_MANAGER_CONF_OPTS += --disable-polkit
>  endif
>  
> -ifeq ($(BR2_PACKAGE_READLINE),y)
> +ifeq ($(BR2_PACKAGE_NETWORK_MANAGER_CLI),y)
>  NETWORK_MANAGER_DEPENDENCIES += readline
>  NETWORK_MANAGER_CONF_OPTS += --with-nmcli
>  else
> -- 
> 2.20.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot