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

[Mike Frysinger <vapier@gentoo.org>]

[-- Attachment #1: Type: text/plain, Size: 2974 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]