Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Trent Piepho <tpiepho@impinj.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH v2] libcurl: Allow selection of TLS package libcurl will use
Date: Fri, 9 Nov 2018 22:53:25 +0000	[thread overview]
Message-ID: <1541804005.30311.392.camel@impinj.com> (raw)
In-Reply-To: <878t217u30.fsf@dell.be.48ers.dk>

On Fri, 2018-11-09 at 23:40 +0100, Peter Korsgaard wrote:
> > > > > > "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:
>  >> +choice
>  >> +	prompt "SSL/TLS library to use"
>  >> +
>  >> +config BR2_PACKAGE_LIBCURL_OPENSSL
>  >> +	bool "OpenSSL"
>  >> +	depends on BR2_PACKAGE_OPENSSL
> 
>  >  I don't really like this depends. I would rather revert it into a select. So:
> 
>  > choice
>  > 	prompt "SSL/TLS library to use"
> 
>  > config BR2_PACKAGE_LIBCURL_OPENSSL
>  > 	bool "openssl"
>  > 	select BR2_PACKAGE_OPENSSL
> 
>  > ...
> 
>  > config BR2_PACKAGE_LIBCURL_NONE
>  > 	bool "disable TLS"
> 
>  > endchoice
> 
>  >  Note that that means that libcurl will by default select openssl, which was not
>  > the case before. However, I think it makes complete sense to default to enabling
>  > TLS support in libcurl. Peter, what do you think? This would obviously have to
>  > be mentioned in the release notes because the behaviour of existing configs
>  > would change.

Don't most optional features in buildroot get auto-enabled when the
package they need is enabled?  Which in effect means the feature is
turned on, not by a setting under the user, but by turning on the
dependency.  I know I've seen this pattern many many times in
buildroot.

Is this pattern of turning on the dependency via an option under the
user of the dependency used elsewhere?  Or would it be a new pattern
unique to libcurl?

> Either that or add a:
> 
>  depends on BR2_PACKAGE_OPENSSL || BR2_PACKAGE_GNUTLS || \
>           BR2_PACKAGE_LIBNSS || BR2_PACKAGE_MBEDTLS
> 
> To the choice option and drop the _LIBCURL_NONE variant. With that we
> have the same behaviour as before, except that you _CAN_ select the TLS
> provider in case multiple providers are available.

I did drop the NONE variant.  Do you mean in Arnout's alternate
example?  It doesn't provide entirely the same behavior.  Example:

openssl off, gnutls on

Current: libcurl uses gnutls
My patch: libcurl uses gnutls
Arnout's proposal: libcurl enables openssl and uses it.  Need to change
config to select gnutls and turn off openssl.
Peter's modification: Same as Arnout's

I believe your modification only produces the same behavior in the case
where all four tls libs are off.  As soon as one turns on, then libcurl
will select openssl.

  reply	other threads:[~2018-11-09 22:53 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-08 22:25 [Buildroot] [PATCH v2] libcurl: Allow selection of TLS package libcurl will use Trent Piepho
2018-11-09 21:56 ` Arnout Vandecappelle
2018-11-09 22:40   ` Peter Korsgaard
2018-11-09 22:53     ` Trent Piepho [this message]
2018-11-09 23:15       ` Peter Korsgaard
2018-11-12 21:20 ` Peter Korsgaard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1541804005.30311.392.camel@impinj.com \
    --to=tpiepho@impinj.com \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox