Re: Include dir intl when building libcommon.a for gdb

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> Date: Wed, 2 Mar 2011 16:14:07 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> 
> > This patch is to fix this problem above.  Cross build gdb for
> > i586-mingw32msvc.  Failure goes away.
> 
> Thanks. I should say up-front that I appreciate your responsiveness.
> It's very nice, particularly in this case where I feel the issue
> is becoming more urgent to fix.
> 
> > gdb/
> > 	* common/Makefile.in: Inherit CC.
> > 	* common/configure.ac (GDB_INCLUDE): Include dir intl.
> > 	* common/configure: Regenerate.
> 
> Now, the bad news - Just this patch alone makes me dislike
> the current approach. I'm really sorry, and maybe that's because
> I'm not understanding all the issues, here.
> 
> > diff --git a/gdb/common/Makefile.in b/gdb/common/Makefile.in
> > index 9230b87..23117a9 100644
> > --- a/gdb/common/Makefile.in
> > +++ b/gdb/common/Makefile.in
> > @@ -26,6 +26,7 @@ COMMON_CPU_OBJ = @COMMON_CPU_OBJ@
> >  # CFLAGS is specifically reserved for setting from the command line
> >  # when running make.  I.E.  "make CFLAGS=-Wmissing-prototypes".
> >  CFLAGS = @CFLAGS@
> > +CC = @CC@
> 
> This part looks fairly obvious, and OK to me.
> 
> > diff --git a/gdb/common/configure.ac b/gdb/common/configure.ac
> > index 1ef85fe..b31a3e9 100644
> > --- a/gdb/common/configure.ac
> > +++ b/gdb/common/configure.ac
> > @@ -63,7 +63,7 @@ if test x"$enable_gdbserver" = xyes; then
> >    GDB_INCLUDE="-I\$(srcdir)/../gdbserver/"
> >  else
> >    GDB_FLAGS=""
> > -  GDB_INCLUDE="-I\$(srcdir)/../ -I\$(BFD_DIR)"
> > +  GDB_INCLUDE="-I\$(srcdir)/../ -I\$(BFD_DIR) -I../../intl/"
> >  fi
> 
> This part, however, makes me uncomfortable.  There are a couple
> of reasons.  The current code is:
> 
>     if test x"$enable_gdbserver" = xyes; then
>       GDB_FLAGS="-DGDBSERVER"
>       GDB_INCLUDE="-I\$(srcdir)/../gdbserver/"
>     else
>       GDB_FLAGS=""
>       GDB_INCLUDE="-I\$(srcdir)/../ -I\$(BFD_DIR)"
>     fi
> 
> The first question is: I don't understand why we have different
> include and compilation flags. I can see how -DGDBSERVER is used
> to select between gdb's "defs.h" and gdbserver's "server.h". So,
> OK for now.  But for the rest, why do we maintain different include
> paths depending on whether we build it for GDB or for GDBserver?
> It's like a circular dependency.  I can explain that we need to
> select either $(srcdir)/../gdbserver/ or $(srcdir)/../ for the
> same reason as above (including "defs.h" or "server.h").
> 
> For the rest, I think that the -I flags should be the same regardless
> of who we build these files for. In my mind, it's supposed to be
> actually independent from both GDB and GDBserver. And unfortunately,
> right now, it is dependent on both :-(.
> 
> This definitely makes me rethink the way we approached the problem.
> By taking what we have now, and moving it to common/, we drag some
> dependencies which I think we do not want. I think we should either
> strive to remove these dependencies as fast as we can, or use an
> approach where we go the other way: Start with the foundations, and
> then implement the things we are trying to move to common/ using
> that foundation.  For instance, defs.h versus server.h.  It's tough
> right now, because defs has more than just definitions. We could
> isolate the part that provides the common non-GDB-specific definitions
> into a common/common-defs.
> 
> In the meantime, one proposed easy way out that doesn't destroy
> all the work that has been done so far is to add all the -I
> directories regardless of who we build libcommon for.  I think
> it makes sense from a conceptual point of view, and it will also
> help us avoid maintaining 2 lists. But maybe it doesn't work for
> practical reasons.

This actually makes me believe that the code that's being moved to
common/ should be virtually #ifdef-free.