Re: [PATCH, V3 10/15] gdb: sim: buildsystem changes to accommodate libsframe

["Jose E. Marchesi" <jose.marchesi@oracle.com>]

> On 02 Nov 2022 20:11, Jose E. Marchesi wrote:
>> 
>> Hi Mike, Indu.
>> 
>> >> > On 30 Oct 2022 00:44, Indu Bhagat via Binutils wrote:
>> >> >> [Changes in V3]
>> >> >>    - Additional diff in sim/ppc/Makefile.in to accommodate libsframe.
>> >> >>      This is needed to ensure --enable-targets=all continues to build.
>> >> >>    - Addressed review comments by Mike Frysinger.
>> >> > 
>> >> > this doesn't seem to actually address my comments.  you're still poking
>> >> > the internals of libtool by accessing files under .libs/.
>> >> 
>> >> gdb does not use libtool yet.
>> >
>> > you have access to the source.  you can change these things.
>> >
>> > also, gdb & sim are sep projects.
>> 
>> I see gdb/configure.ac uses the same strategy in order to locate the
>> in-tree libbacktrace.a and libctf:
>> 
>>   if test "${enable_libbacktrace}" = "yes"; then
>>     LIBBACKTRACE_INC="-I$srcdir/../libbacktrace/ -I../libbacktrace/"
>>     LIBBACKTRACE_LIB=../libbacktrace/.libs/libbacktrace.a
>>     AC_DEFINE(HAVE_LIBBACKTRACE, 1, [Define if libbacktrace is being used.])
>>   else
>>     LIBBACKTRACE_INC=
>>     LIBBACKTRACE_LIB=
>>   fi
>> 
>>   [...]
>> 
>>   if test x${enable_static} = xno; then
>>     LIBCTF="-Wl,--rpath,../libctf/.libs ../libctf/.libs/libctf.so"
>>     CTF_DEPS="../libctf/.libs/libctf.so"
>>   else
>>     LIBCTF="../libctf/.libs/libctf.a"
>>     CTF_DEPS="$LIBCTF"
>>   fi
>> 
>> With corresponding substitutions in gdb/Makefile.in.
>> 
>> I agree it would be better to have GDB libtoolized so it could refer to
>> the .la libraries directly thus avoiding internals, but could that be
>> done in a separated patch set, also covering the other cases?
>> 
>> In the meanwhile, Indu could change her patch in order to look for
>> libsframe.so in gdb/configure.ac instead of gdb/Makefile.in, as it is
>> done for the other libs.  Then we libtoolize.
>
> "the code is already in bad shape, so let's add more kindle to the fire"
> isn't a great strategy.  hoping someone else will come and clean up the
> mess also isn't a great strategy ... usually that means it never gets
> cleaned up, and the tech debt just continues to build.  so "let's do
> this as a followup" almost always translates into "i don't want to do
> it, and it's never actually going to happen, so let me merge anyways".
> i'm not saying that's necessarily the intention of the person making
> such a request, just that that's the practical result in my experience
> in the vast majority of cases.  people, no matter how well intentioned,
> are busy, so without any pressing leverage (like "this is required if
> you want to merge"), it never improves.
>
> to be clear, i'm not a global gdb maintainer, so if you can convince
> one of them, then certainly they override.  i am NAKing adding any
> such hacks to the sim code though.  although that's a bit moot since
> i've already posted patches to clean up its libtool usage which means
> it doesn't need any changes for libsframe logic.
> -mike

I'm following up in gdb-patches@.