Re: [PATCH] binutils, gdb: support zstd compressed debug sections

[Fangrui Song <maskray@google.com>]

On 2022-09-19, Simon Marchi wrote:
>> diff --git a/config/zstd.m4 b/config/zstd.m4
>> new file mode 100644
>> index 00000000000..b8b3a199730
>> --- /dev/null
>> +++ b/config/zstd.m4
>> @@ -0,0 +1,12 @@
>> +AC_DEFUN([AM_ZSTD],
>> +[
>> +  zstdlib=
>> +  AC_ARG_WITH(system-zstd,
>> +  [AS_HELP_STRING([--with-system-zstd], [use installed zstd])],
>> +  if test x$with_system_zstd = xyes ; then
>> +    zstdlib=-lzstd
>> +    AC_DEFINE(HAVE_ZSTD_H)
>> +  fi
>> +  )
>> +  AC_SUBST(zstdlib)
>> +])
>
>Hi Fangrui,
>
>I understand that you did this by copying zlib here.  However, the only
>reason we have a --use-system-zlib is because the binutils-gdb contains
>a copy of zlib, which is used by default, and --use-system-zlib can be
>used to prefer the system version.  Since we don't carry a copy of zstd
>in the tree, I don't think we need --with-system-zstd.
>
>However, since zstd ships with a .pc file, my preference would be to use
>PKG_CHECK_MODULES to have pkg-config find the right flags, and have
>--with/--without-zstd.  Same as I did for the msgpack library here:
>
>  https://gitlab.com/gnutools/binutils-gdb/-/commit/2952f10cd79af4645222f124f28c7928287d8113
>
>I am not a binutils maintainers though (just GDB), so in the end it's up
>to the binutils maintainers to decide and GDB will probably follow suit.

Thanks.  I see --with-msgpack and --with-jansson, so --with-zstd is
probably a good option name.

>> diff --git a/configure.ac b/configure.ac
>> index a5555a9c91b..f7316b24ac6 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -246,6 +246,9 @@ if test x$with_system_zlib = xyes ; then
>>    noconfigdirs="$noconfigdirs zlib"
>>  fi
>>
>> +AC_ARG_WITH(system-zstd,
>> +[AS_HELP_STRING([--with-system-zstd], [use installed zstd])])
>> +
>
>I don't think you need this change.  --with-system-zlib exists here
>because we need to decide whether or not to build the local zlib.  But
>since we don't have a local zstd...

I am unfamilir with autotools, but I agree that pkg-config is better.  Changed to PKG_CHECK_MODULES
and pushed the change to
https://gitlab.com/MaskRay/binutils-gdb/-/commits/zstd

However, now `make all-gdb` fails to link because gdb/Makefile does not have `ZSTD_LIBS = -lzstd` ...

>Simon
>