Re: [PATCH v2 0/6] Move gdbsupport to top level

[Pedro Alves <palves at redhat dot com>]

On 1/16/20 2:54 AM, Christian Biesinger via gdb-patches wrote:
> On Wed, Jan 15, 2020 at 5:12 PM Pedro Alves <palves@redhat.com> wrote:
>>
>> On 1/15/20 9:46 PM, Pedro Alves wrote:
>>> On 1/15/20 9:35 PM, Pedro Alves wrote:
>>>> On 1/15/20 8:23 PM, Pedro Alves wrote:
>>>>> On 1/15/20 2:55 PM, Pedro Alves wrote:
>>>>>> On 1/15/20 2:41 PM, Pedro Alves wrote:
>>>>>>> Don't know what I think of gnulib headers including <config.h>.
>>>>>>> Maybe we should rename gdb's config.h to gdb-config.h too.
>>>>>>
>>>>>> Hit reply to soon.  I meant to add,
>>>>>>
>>>>>> ... and then, add a manually-written config.h in the build
>>>>>> dir that does:
>>>>>>
>>>>>>  #include <gdbsupport/support-config.h>
>>>>>>  #include <gdb-config.h>
>>>>>>
>>>>>> We'd do the same to gdbsupport, add a config.h in its
>>>>>> build dir that does:
>>>>>>
>>>>>>  #include "gnulib/config.h"
>>>>>>  #include <support-config.h>
>>>>>>
>>>>>> Those config.h files would go in the build dirs so that
>>>>>> they're not picked by other build directories.
>>>>>>
>>>>>> With that, any "#include <config.h>" in any header ends up
>>>>>> picking the currently-being-built project's config.h, plus
>>>>>> the dependencies' config.h files.
>>>>>>
>>>>>> Just a half-baked thought.  Not sure it's the best idea.
>>>>>
>>>>> I tried it and it seems to work OK.  Fixes the build at least.
>>>>> Still not sure it's the best idea.  WDYT?
>>>>>
>>>>> The patch is actually quite small, but since I've rename
>>>>> config.h -> gdb-config.h etc., and _then_ added new config.h
>>>>> files, git doesn't notice the renames.
>>>>>
>>>>> I wonder whether there's anything could do to stop gnulib and
>>>>> gdbsupport's configure from defining PACKAGE_NAME etc. in their
>>>>> generated config.h files.
>>>>>
>>>> Here's an improved version, which fixes gdbserver's standalone
>>>> build, simplifies gdbsupport's config.h (there's no need for
>>>> #ifdef GDBSERVER stuff since gdbserver doesn't use gdbsupport
>>>> as a library yet), and adds copyright/intro comments.
>>>>
>>>
>>> I put this in users/palves/config.h if you want to play with it.
>>
>> I've also pushed a patch there to fix the missing -std=gnu++11 issue.
> 
> For that patch, why not just use AM_CFLAGS/AM_CXXFLAGS?

I was mainly following what GDB does, with:

 COMPILE.pre = $(CXX) -x c++ $(CXX_DIALECT)

I guess I was the one who did that, so that's not going to be
a strong explanation.  :-)

It's just that I feel like the compiler mode is more about
picking a different compiler than about picking some flags
like warnings flags or compilation levels.  I think that
in practice the only difference is that you see the
-std=gnu++11 next to the compiler command:

$ rm -f selftest.o && make V=1 selftest.o
/opt/gcc-4.8/bin/g++ -std=gnu++11 -DHAVE_CONFIG_H -I. -I/home/pedro/gdb/binutils-gdb/src/gdbsupport  -I/home/pedro/gdb/binutils-gdb/src/gdbsupport/config.h -I/home/pedro/gdb/binutils-gdb/src/gdbsupport/../include -I/home/pedro/gdb/binutils-gdb/src/gdbsupport/../gdb -I../gnulib/import -I/home/pedro/gdb/binutils-gdb/src/gdbsupport/../gnulib/import -I.. -I/home/pedro/gdb/binutils-gdb/src/gdbsupport/..  -I../bfd -I/home/pedro/gdb/binutils-gdb/src/gdbsupport/../bfd   -g3 -O0 -MT selftest.o -MD -MP -MF .deps/selftest.Tpo -c -o selftest.o /home/pedro/gdb/binutils-gdb/src/gdbsupport/selftest.c
mv -f .deps/selftest.Tpo .deps/selftest.Po

vs, with AM_CFLAGS:

$ rm -f selftest.o && make V=1 selftest.o
/opt/gcc-4.8/bin/g++ -DHAVE_CONFIG_H -I. -I/home/pedro/gdb/binutils-gdb/src/gdbsupport  -I/home/pedro/gdb/binutils-gdb/src/gdbsupport/config.h -I/home/pedro/gdb/binutils-gdb/src/gdbsupport/../include -I/home/pedro/gdb/binutils-gdb/src/gdbsupport/../gdb -I../gnulib/import -I/home/pedro/gdb/binutils-gdb/src/gdbsupport/../gnulib/import -I.. -I/home/pedro/gdb/binutils-gdb/src/gdbsupport/..  -I../bfd -I/home/pedro/gdb/binutils-gdb/src/gdbsupport/../bfd  -std=gnu++11 -g3 -O0 -MT selftest.o -MD -MP -MF .deps/selftest.Tpo -c -o selftest.o /home/pedro/gdb/binutils-gdb/src/gdbsupport/selftest.c
mv -f .deps/selftest.Tpo .deps/selftest.Po

AFAICT, overriding CXXFLAGS or CXX in the make invocation works
the same in either case.

Thanks,
Pedro Alves