Skip to contentSkip to conversation

package/network-manager: add optional nmcli support

PATCH Superseded Michael Fischer 3 participants 5 messages Jan 26, 2022 – Aug 21, 2022

lore
Share & tools
lore

Michael Fischer

author
#0 lore raw

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

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

Signed-off-by Michael Fischer <[email protected]>
---
 package/network-manager/Config.in          | 7 ++++++-
 package/network-manager/network-manager.mk | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)
package/network-manager/Config.in +6 -1
index a48cb37b15..1a123a26e2 100644
@@ -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"
package/network-manager/network-manager.mk +1 -1
index 974320fce0..1eb9233b44 100644
@@ -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
Signature · 6 lines
2.20.1

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#1 lore raw

Hello,

+Yann on legacy handling (see below)

Quoted 1 line · On Wed, 26 Jan 2022 10:04:57 +0100 Michael Fischer <[email protected]> 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.
"""

package/network-manager/Config.in config BR2_PACKAGE_NETWORK_MANAGER_TUI · line 44

Quoted 8 lines
Signed-off-by Michael Fischer <[email protected]>
package/network-manager/Config.in +5 -0
+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

Signature · 7 lines
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#2 lore raw

Thomas, Michael, All,

On 2022-01-26 23:14 +0100, Thomas Petazzoni spake thusly:

Quoted 2 lines

On Wed, 26 Jan 2022 10:04:57 +0100
Michael Fischer <[email protected]> wrote:

[--SNIP--]

Quoted 6 lines

package/network-manager/Config.in config BR2_PACKAGE_NETWORK_MANAGER_TUI · line 44

Quoted 5 lines
package/network-manager/Config.in +5 -0
+config BR2_PACKAGE_NETWORK_MANAGER_CLI
+        bool "nmcli support"
+        select BR2_PACKAGE_READLINE
+        help
+          This option enables support for NetworkManager Command Line Interface
Quoted 7 lines

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.

Quoted 10 lines

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.

Signature · 10 lines
.-----------------.--------------------.------------------.--------------------.
|  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
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
author
#3 lore raw

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

In reply to Thomas Petazzoni

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 <[email protected]>
---
 package/network-manager/Config.in          | 8 +++++++-
 package/network-manager/network-manager.mk | 2 +-
 2 files changed, 8 insertions(+), 2 deletions(-)
package/network-manager/Config.in +7 -1
index a48cb37b15..f61a046a95 100644
@@ -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"
package/network-manager/network-manager.mk +1 -1
index 974320fce0..1eb9233b44 100644
@@ -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
Signature · 6 lines
2.20.1

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
Applied
#4 lore raw

Michael, All,

On 2022-01-27 11:46 +0100, Michael Fischer spake thusly:

Quoted 6 lines

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 <[email protected]>

Applied to master^Wnext, thanks.

Regards,
Yann E. MORIN.

Quoted 51 lines
---
 package/network-manager/Config.in          | 8 +++++++-
 package/network-manager/network-manager.mk | 2 +-
 2 files changed, 8 insertions(+), 2 deletions(-)
package/network-manager/Config.in +7 -1
index a48cb37b15..f61a046a95 100644
@@ -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"
package/network-manager/network-manager.mk +1 -1
index 974320fce0..1eb9233b44 100644
@@ -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
Signature · 6 lines
2.20.1

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
Signature · 10 lines
.-----------------.--------------------.------------------.--------------------.
|  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
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot