diff mbox series

curl: don't enable debug builds

Message ID 20230111141753.1249071-1-ross.burton@arm.com
State New
Headers show
Series curl: don't enable debug builds | expand

Commit Message

Ross Burton Jan. 11, 2023, 2:17 p.m. UTC
In oe-core 27824261 --enable-debug was added to the configure arguments
to turn on debugging symbols.  However, enabling debug mode does more
than turn on debugging symbols and introduces some codepaths that can be
controlled with environment variables.  Bluntly, the curl maintainer
says that --enable-debug should not be used in production:

https://curl.se/mail/lib-2023-01/0039.html

I did a build and verified that the curl-dbg package doesn't massively
shrink, so the debug symbols are still being built.

Remove the debug options and hide them behind a PACKAGECONFIG, with a
comment that it should not be used in production.

Signed-off-by: Ross Burton <ross.burton@arm.com>
---
 meta/recipes-support/curl/curl_7.87.0.bb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Quentin Schulz Jan. 11, 2023, 2:46 p.m. UTC | #1
Hi Ross,

On 1/11/23 15:17, Ross Burton wrote:
> In oe-core 27824261 --enable-debug was added to the configure arguments
> to turn on debugging symbols.  However, enabling debug mode does more
> than turn on debugging symbols and introduces some codepaths that can be
> controlled with environment variables.  Bluntly, the curl maintainer
> says that --enable-debug should not be used in production:
> 
> https://urldefense.com/v3/__https://curl.se/mail/lib-2023-01/0039.html__;!!OOPJP91ZZw!jMBUWN09d9qh5ERXzeSectyn9T7uX0vl50AFyD3lmFEPTECVwbRqWi6LWeV2NdnKZMWF2rZtDMtAe08Lsb_e3phBQgWpikZIzA$
> 
> I did a build and verified that the curl-dbg package doesn't massively
> shrink, so the debug symbols are still being built.
> 
> Remove the debug options and hide them behind a PACKAGECONFIG, with a
> comment that it should not be used in production.
> 
> Signed-off-by: Ross Burton <ross.burton@arm.com>
> ---
>   meta/recipes-support/curl/curl_7.87.0.bb | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/recipes-support/curl/curl_7.87.0.bb b/meta/recipes-support/curl/curl_7.87.0.bb
> index a3ad66f1aa3..9956a284228 100644
> --- a/meta/recipes-support/curl/curl_7.87.0.bb
> +++ b/meta/recipes-support/curl/curl_7.87.0.bb
> @@ -32,6 +32,8 @@ PACKAGECONFIG:class-nativesdk = "ipv6 openssl proxy random threaded-resolver ver
>   PACKAGECONFIG[ares] = "--enable-ares,--disable-ares,c-ares,,,threaded-resolver"
>   PACKAGECONFIG[brotli] = "--with-brotli,--without-brotli,brotli"
>   PACKAGECONFIG[builtinmanual] = "--enable-manual,--disable-manual"
> +# Don't use this in production
> +PACKAGECONFIG[debug] = "--enable-debug,--disable-debug"

Should we have --disable-curldebug in the enable path here? I see you 
remove it further down but it's not here.

Cheers,
Quentin
Ross Burton Jan. 11, 2023, 6:03 p.m. UTC | #2
On 11 Jan 2023, at 14:46, Quentin Schulz <quentin.schulz@theobroma-systems.com> wrote:
> Should we have --disable-curldebug in the enable path here? I see you remove it further down but it's not here.

No need:

    default)
      dnl configure's curldebug option not specified. Initially we will
      dnl handle this as a request to use the same setting as option
      dnl --enable-debug. IOW, initially, for debug-enabled builds
      dnl this will be handled as a request to enable curldebug if
      dnl possible, and for debug-disabled builds this will be handled
      dnl as a request to disable curldebug.

Ross
diff mbox series

Patch

diff --git a/meta/recipes-support/curl/curl_7.87.0.bb b/meta/recipes-support/curl/curl_7.87.0.bb
index a3ad66f1aa3..9956a284228 100644
--- a/meta/recipes-support/curl/curl_7.87.0.bb
+++ b/meta/recipes-support/curl/curl_7.87.0.bb
@@ -32,6 +32,8 @@  PACKAGECONFIG:class-nativesdk = "ipv6 openssl proxy random threaded-resolver ver
 PACKAGECONFIG[ares] = "--enable-ares,--disable-ares,c-ares,,,threaded-resolver"
 PACKAGECONFIG[brotli] = "--with-brotli,--without-brotli,brotli"
 PACKAGECONFIG[builtinmanual] = "--enable-manual,--disable-manual"
+# Don't use this in production
+PACKAGECONFIG[debug] = "--enable-debug,--disable-debug"
 PACKAGECONFIG[dict] = "--enable-dict,--disable-dict,"
 PACKAGECONFIG[gnutls] = "--with-gnutls,--without-gnutls,gnutls"
 PACKAGECONFIG[gopher] = "--enable-gopher,--disable-gopher,"
@@ -68,9 +70,7 @@  EXTRA_OECONF = " \
     --enable-crypto-auth \
     --with-ca-bundle=${sysconfdir}/ssl/certs/ca-certificates.crt \
     --without-libpsl \
-    --enable-debug \
     --enable-optimize \
-    --disable-curldebug \
     ${@'--without-ssl' if (bb.utils.filter('PACKAGECONFIG', 'gnutls mbedtls nss openssl', d) == '') else ''} \
 "