libcurl: enable mbedtls support
PATCH 1/1 libcurl: enable mbedtls support
Now that we've got an mbedtls package in the tree we can enable the optional support for it in libcurl.
--- package/libcurl/libcurl.mk | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
package/libcurl/libcurl.mk +4 -2
@@ -40,10 +40,12 @@ else ifeq ($(BR2_PACKAGE_LIBNSS),y) LIBCURL_CONF_OPTS += --with-nss=$(STAGING_DIR)/usr LIBCURL_CONF_ENV += CPPFLAGS="$(TARGET_CPPFLAGS) `$(PKG_CONFIG_HOST_BINARY) nspr nss --cflags`" LIBCURL_DEPENDENCIES += libnss +else ifeq ($(BR2_PACKAGE_MBEDTLS),y) +LIBCURL_CONF_OPTS += --with-mbedtls=$(STAGING_DIR)/usr +LIBCURL_DEPENDENCIES += mbedtls else -# polarssl support needs 1.3.x LIBCURL_CONF_OPTS += --without-ssl --without-gnutls \ - --without-polarssl --without-nss + --without-polarssl --without-nss --without-mbedtls endif ifeq ($(BR2_PACKAGE_C_ARES),y)
Signature · 1 line
2.4.10
PATCH 1/1 libcurl: enable mbedtls support
Hi Gustavo,
Quoted 13 lines · Gustavo Zacarias
Now that we've got an mbedtls package in the tree we can enable the optional support for it in libcurl.
Signed-off-by Gustavo Zacarias <[email protected]>--- package/libcurl/libcurl.mk | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)package/libcurl/libcurl.mk +0
-0
package/libcurl/libcurl.mk else ifeq ($(BR2_PACKAGE_LIBNSS),y) · line 44
Quoted 8 lines
package/libcurl/libcurl.mk +3
-1@@ -40,10 +40,12 @@ else ifeq ($(BR2_PACKAGE_LIBNSS),y) LIBCURL_CONF_OPTS += --with-nss=$(STAGING_DIR)/usr LIBCURL_CONF_ENV += CPPFLAGS="$(TARGET_CPPFLAGS) `$(PKG_CONFIG_HOST_BINARY) nspr nss --cflags`" LIBCURL_DEPENDENCIES += libnss +else ifeq ($(BR2_PACKAGE_MBEDTLS),y) +LIBCURL_CONF_OPTS += --with-mbedtls=$(STAGING_DIR)/usr +LIBCURL_DEPENDENCIES += mbedtls else -# polarssl support needs 1.3.x
This is unrelated to your change, it should be a separate patch. But anyway I don't understand why it should be removed: we still package polarssl 1.2.18, so the comment should still apply. Am I missing something?
With that fixed (or explained):
And:
[Built with/without mbedtls, ran on qemu, checked that 'curl -V' shows the SSL only with mbedtls]
Signature · 1 line
Luca
PATCH 1/1 libcurl: enable mbedtls support
Quoted 5 lines · Luca Ceresoli
This is unrelated to your change, it should be a separate patch. But anyway I don't understand why it should be removed: we still package polarssl 1.2.18, so the comment should still apply. Am I missing something?
Quoted 7 lines
With that fixed (or explained):
Reviewed-by Luca Ceresoli <[email protected]>And:
[Built with/without mbedtls, ran on qemu, checked that 'curl -V' shows the SSL only with mbedtls]Tested-by Luca Ceresoli <[email protected]>
Hi.
As you know polarssl was renamed to mbedtls when bought by ARM, this was circa the 1.3.x release.
Older 1.3.x releases install the library as polarssl alone, newer ones install dual as polarssl with symlinks to mbedtls (might be the other way around, i don't recall at the moment - it doesn't matter anyway), so first the distinction between polarssl and mbedtls is terribly blurred for the 1.3.x series.
Incidentally this is what makes polarssl 1.2.x being able to live side-by-side with mbedtls (2.x, which doesn't keep legacy handling) (i believe this is more product of branding/accident than careful thinking). So now in some cases polarssl means 1.2.x, in other cases it means mbedtls 1.3.x, or the new version because nobody cared to rename everything. Basically any explanation given out of context is useless without historical context.
Making 1.3.x live side-by-side with mbedtls (let's call it that way for 2+) is probably possible by ditching the new naming and just keeping compat, i had a look but i think it was a dead end anyway, this requires patching the build of mbedtls/polarssl 1.3.x somewhat and that wouldn't be accepted upstream.
Regards.
PATCH 1/1 libcurl: enable mbedtls support
Hi Gustavo,
Quoted 28 lines · Gustavo Zacarias
Quoted 5 lines · Luca Ceresoli
This is unrelated to your change, it should be a separate patch. But anyway I don't understand why it should be removed: we still package polarssl 1.2.18, so the comment should still apply. Am I missing something?
Quoted 7 lines
With that fixed (or explained):
Reviewed-by Luca Ceresoli <[email protected]>And:
[Built with/without mbedtls, ran on qemu, checked that 'curl -V' shows the SSL only with mbedtls]Tested-by Luca Ceresoli <[email protected]>Hi.
As you know polarssl was renamed to mbedtls when bought by ARM, this was circa the 1.3.x release.
Older 1.3.x releases install the library as polarssl alone, newer ones install dual as polarssl with symlinks to mbedtls (might be the other way around, i don't recall at the moment - it doesn't matter anyway), so first the distinction between polarssl and mbedtls is terribly blurred for the 1.3.x series.
Incidentally this is what makes polarssl 1.2.x being able to live side-by-side with mbedtls (2.x, which doesn't keep legacy handling) (i believe this is more product of branding/accident than careful thinking). So now in some cases polarssl means 1.2.x, in other cases it means mbedtls 1.3.x, or the new version because nobody cared to rename
Quoted 8 lines
everything.
Basically any explanation given out of context is useless without historical context.
Making 1.3.x live side-by-side with mbedtls (let's call it that way for 2+) is probably possible by ditching the new naming and just keeping compat, i had a look but i think it was a dead end anyway, this requires patching the build of mbedtls/polarssl 1.3.x somewhat and that wouldn't be accepted upstream.
Thank you for the explanation. Indeed, it got me totally puzzled! I am no mbedtls/polarssl expert, I never used them...
If I understand correctly:
- without this patch, the comment states that you'd need to upgrade polarssl to 1.3.x to have it supported in libcurl
- with the patch libcurl uses mbedtls, which makes polarssl uninteresting.
Is it correct?
Signature · 1 line
Luca
PATCH 1/1 libcurl: enable mbedtls support
Quoted 10 lines · Luca Ceresoli
Thank you for the explanation. Indeed, it got me totally puzzled! I am no mbedtls/polarssl expert, I never used them...
If I understand correctly:
- without this patch, the comment states that you'd need to upgrade polarssl to 1.3.x to have it supported in libcurl
- with the patch libcurl uses mbedtls, which makes polarssl uninteresting.Is it correct?
Hi Luca.
Yes, the polarssl comment becomes redundant, since we can't support our current 1.2.x and we can't (easily) upgrade to 1.3.x, which i think would be pointless.
Newer openvpn can support 1.3.x but not newer, i know git master which is very much in a -dev state can do 2.x but i wouldn't pick that up lightly. Regarding shairport-sync i don't know if it can work with newer versions, maybe it can do mbedtls directly now that it's been bumped. Regards.
PATCH 1/1 libcurl: enable mbedtls support
Hi Gostavo,
Quoted 13 lines · Gustavo Zacarias
Quoted 10 lines · Luca Ceresoli
Thank you for the explanation. Indeed, it got me totally puzzled! I am no mbedtls/polarssl expert, I never used them...
If I understand correctly:
- without this patch, the comment states that you'd need to upgrade polarssl to 1.3.x to have it supported in libcurl
- with the patch libcurl uses mbedtls, which makes polarssl uninteresting.Is it correct?
Quoted 9 lines
Hi Luca.
Yes, the polarssl comment becomes redundant, since we can't support our current 1.2.x and we can't (easily) upgrade to 1.3.x, which i think would be pointless.
Newer openvpn can support 1.3.x but not newer, i know git master which is very much in a -dev state can do 2.x but i wouldn't pick that up lightly.
Regarding shairport-sync i don't know if it can work with newer versions, maybe it can do mbedtls directly now that it's been bumped.
I see, thanks for the additional clarification. Now it makes sense.
Since it is non-obvious (at least it wasn't to me), it's worth adding a comment, such as: "mbedtls is a successor or polarssl, thus polarssl support is useless".
Care to resend with that?
Thanks,
Signature · 1 line
Luca
PATCH 1/1 libcurl: enable mbedtls support
Quoted 9 lines · Luca Ceresoli
I see, thanks for the additional clarification. Now it makes sense.
Since it is non-obvious (at least it wasn't to me), it's worth adding a comment, such as: "mbedtls is a successor or polarssl, thus polarssl support is useless".
Care to resend with that?
Thanks,
Does it make any sense?
It never had any polarssl/mbedtls support to begin with. Regards.
PATCH 1/1 libcurl: enable mbedtls support
Hi Gustavo,
Quoted 8 lines · Gustavo Zacarias
Quoted 6 lines · Luca Ceresoli
I see, thanks for the additional clarification. Now it makes sense.
Since it is non-obvious (at least it wasn't to me), it's worth adding a comment, such as: "mbedtls is a successor or polarssl, thus polarssl support is useless".
Quoted 6 lines
Quoted 3 lines
Care to resend with that?
Thanks,
Does it make any sense?
It never had any polarssl/mbedtls support to begin with.
Well, it makes sense in the commit log IMO. It would have replied my doubts immediately.
Signature · 1 line
Luca
PATCH 1/1 libcurl: enable mbedtls support
Quoted 2 lines · Luca Ceresoli
Well, it makes sense in the commit log IMO. It would have replied my doubts immediately.
Which is now documented in the ML / archive :)
Regards.
PATCH 1/1 libcurl: enable mbedtls support
In reply to Gustavo ZacariasDear Gustavo Zacarias,
Quoted 7 lines · Gustavo Zacarias
Now that we've got an mbedtls package in the tree we can enable the optional support for it in libcurl.
Signed-off-by Gustavo Zacarias <[email protected]>--- package/libcurl/libcurl.mk | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Applied with an improved commit log, as suggested by Luca.
Gustavo, for the record, I do agree with Luca that an additional explanation was needed. It is a pity that you refused to send an updated version, and simply referred to the ML archives as a way of explaining your patches.
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com