diff mbox series

[6/7] libnetconf2: adjust dependencies

Message ID 8c74f9fdebe7eef326420f5354f3ec8163930074.1575456104.git.jan.kundrat@cesnet.cz
State Changes Requested
Headers show
Series Improve sysrepo support | expand

Commit Message

Jan Kundrát March 17, 2017, 6:29 p.m. UTC
This package needs at least one of these two options, or both:

- openssl
- libssh + the server option

Without these, it is not possible to produce a usable library, so let's
reflect this in the dependencies.

I'm not sure that there is so much value in this; I would actually
prefer to have both of these unconditionally enabled, but the package is
already done in this way, so be it.

Signed-off-by: Jan Kundrát <jan.kundrat@cesnet.cz>
---
 package/libnetconf2/Config.in | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Heiko Thiery Dec. 4, 2019, 12:53 p.m. UTC | #1
Hi Jan,

> -----Ursprüngliche Nachricht-----
> Von: Jan Kundrát [mailto:jan.kundrat@cesnet.cz]
> Gesendet: Freitag, 17. März 2017 19:30
> An: buildroot@buildroot.org
> Cc: Heiko Thiery; Fabrice Fontaine
> Betreff: [PATCH 6/7] libnetconf2: adjust dependencies
> 
> This package needs at least one of these two options, or both:
> 
> - openssl
> - libssh + the server option
> 
> Without these, it is not possible to produce a usable library, so let's reflect this
> in the dependencies.
> 
> I'm not sure that there is so much value in this; I would actually prefer to have
> both of these unconditionally enabled, but the package is already done in this
> way, so be it.

The intention was to be able to select the SSH/TLS support by providing/selecting the
dependent package.

Do you think that is not reasonable?

> Signed-off-by: Jan Kundrát <jan.kundrat@cesnet.cz>
> ---
>  package/libnetconf2/Config.in | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/package/libnetconf2/Config.in b/package/libnetconf2/Config.in
> index 0a735b5ed6..6d4c9c71d1 100644
> --- a/package/libnetconf2/Config.in
> +++ b/package/libnetconf2/Config.in
> @@ -3,6 +3,7 @@ config BR2_PACKAGE_LIBNETCONF2
>  	depends on BR2_TOOLCHAIN_HAS_THREADS
>  	depends on !BR2_STATIC_LIBS
>  	depends on BR2_USE_MMU
> +	depends on BR2_PACKAGE_LIBSSH_SERVER || BR2_PACKAGE_OPENSSL
>  	select BR2_PACKAGE_LIBYANG
>  	help
>  	  libnetconf2 is a NETCONF library in C intended for building @@ -13,3
> +14,6 @@ config BR2_PACKAGE_LIBNETCONF2  comment "libnetconf2 needs a
> toolchain w/ threads, dynamic libraray"
>  	depends on BR2_USE_MMU
>  	depends on BR2_STATIC_LIBS || !BR2_TOOLCHAIN_HAS_THREADS
> +
> +comment "libnetconf2 requires libssh w/ server support and/or openssl"
> +	depends on !BR2_PACKAGE_LIBSSH_SERVER &&
> !BR2_PACKAGE_OPENSSL
> --
> 2.21.0
>
Jan Kundrát Dec. 4, 2019, 1 p.m. UTC | #2
> The intention was to be able to select the SSH/TLS support by 
> providing/selecting the
> dependent package.
>
> Do you think that is not reasonable?

I think that the code in current buildroot does not enforce this. I.e., in 
case I have neither openssl nor libssh+server already selected, I will end 
up with a libnetconf2 which cannot do anything. I think that that is 
confusing.

There are other options, of course:

- We could add nested suboptions below BR2_PACKAGE_LIBNETCONF2 which would 
pick  the corresponding depenency. That way, people would at least have a 
hint that they should drill down within KConfig and notice that there are 
two options. Both of these could be checked by default. Disadvantage: if 
the libssh+server or openssl gets enabled later, there's a risk that these 
two get out of sync.

- We coud just add a hard dependency on both. That IMHO also makes sense 
because I have a feeling that the majority of people actually want both 
ways.

Jan
Heiko Thiery Dec. 4, 2019, 1:11 p.m. UTC | #3
> -----Ursprüngliche Nachricht-----
> Von: Jan Kundrát [mailto:jan.kundrat@cesnet.cz]
> Gesendet: Mittwoch, 4. Dezember 2019 14:01
> An: Heiko Thiery
> Cc: buildroot@buildroot.org; Fabrice Fontaine
> Betreff: Re: [Buildroot] [PATCH 6/7] libnetconf2: adjust dependencies
> 
> > The intention was to be able to select the SSH/TLS support by
> > providing/selecting the dependent package.
> >
> > Do you think that is not reasonable?
> 
> I think that the code in current buildroot does not enforce this. I.e., in case I
> have neither openssl nor libssh+server already selected, I will end up with a
> libnetconf2 which cannot do anything. I think that that is confusing.
> 
> There are other options, of course:
> 
> - We could add nested suboptions below BR2_PACKAGE_LIBNETCONF2 which
> would pick  the corresponding depenency. That way, people would at least have
> a hint that they should drill down within KConfig and notice that there are two
> options. Both of these could be checked by default. Disadvantage: if the
> libssh+server or openssl gets enabled later, there's a risk that these two get out
> of sync.
> 
> - We coud just add a hard dependency on both. That IMHO also makes sense
> because I have a feeling that the majority of people actually want both ways.

Just tried to add you're patch and see that with that at least the package testing (utils/test-pkg -p sysrep) will not work without providing a config-snippet that enables one of them (ssl or ssh).

@thomas: is that a valid way (package cannot be tested without config-snippet) or do we have to select the required dependencies to have the test abilitiy?

> Jan
Thomas Petazzoni Dec. 4, 2019, 3:14 p.m. UTC | #4
On Fri, 17 Mar 2017 19:29:53 +0100
Jan Kundrát <jan.kundrat@cesnet.cz> wrote:

> This package needs at least one of these two options, or both:
> 
> - openssl
> - libssh + the server option
> 
> Without these, it is not possible to produce a usable library, so let's
> reflect this in the dependencies.
> 
> I'm not sure that there is so much value in this; I would actually
> prefer to have both of these unconditionally enabled, but the package is
> already done in this way, so be it.
> 
> Signed-off-by: Jan Kundrát <jan.kundrat@cesnet.cz>
> ---
>  package/libnetconf2/Config.in | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/package/libnetconf2/Config.in b/package/libnetconf2/Config.in
> index 0a735b5ed6..6d4c9c71d1 100644
> --- a/package/libnetconf2/Config.in
> +++ b/package/libnetconf2/Config.in
> @@ -3,6 +3,7 @@ config BR2_PACKAGE_LIBNETCONF2
>  	depends on BR2_TOOLCHAIN_HAS_THREADS
>  	depends on !BR2_STATIC_LIBS
>  	depends on BR2_USE_MMU
> +	depends on BR2_PACKAGE_LIBSSH_SERVER || BR2_PACKAGE_OPENSSL

If one of these is really mandatory for the package to work in any
meaningful way, then:

	select ONE_OPTION if !SECOND_OPTION

is preferred, so that it is more natural for users, and you don't need
the Config.in comment.

Thomas
diff mbox series

Patch

diff --git a/package/libnetconf2/Config.in b/package/libnetconf2/Config.in
index 0a735b5ed6..6d4c9c71d1 100644
--- a/package/libnetconf2/Config.in
+++ b/package/libnetconf2/Config.in
@@ -3,6 +3,7 @@  config BR2_PACKAGE_LIBNETCONF2
 	depends on BR2_TOOLCHAIN_HAS_THREADS
 	depends on !BR2_STATIC_LIBS
 	depends on BR2_USE_MMU
+	depends on BR2_PACKAGE_LIBSSH_SERVER || BR2_PACKAGE_OPENSSL
 	select BR2_PACKAGE_LIBYANG
 	help
 	  libnetconf2 is a NETCONF library in C intended for building
@@ -13,3 +14,6 @@  config BR2_PACKAGE_LIBNETCONF2
 comment "libnetconf2 needs a toolchain w/ threads, dynamic libraray"
 	depends on BR2_USE_MMU
 	depends on BR2_STATIC_LIBS || !BR2_TOOLCHAIN_HAS_THREADS
+
+comment "libnetconf2 requires libssh w/ server support and/or openssl"
+	depends on !BR2_PACKAGE_LIBSSH_SERVER && !BR2_PACKAGE_OPENSSL