Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Kai Tietz <ktietz70@googlemail.com>
To: Joel Brobecker <brobecker@adacore.com>
Cc: Yao Qi <yao@codesourcery.com>, gdb-patches@sourceware.org
Subject: Re: Include dir intl when building libcommon.a for gdb
Date: Wed, 02 Mar 2011 12:34:00 -0000	[thread overview]
Message-ID: <AANLkTi=Z1kXKyQp5wPv3eNJPg1pohyESmyJMaeSN1=Tf@mail.gmail.com> (raw)
In-Reply-To: <20110302121407.GO30306@adacore.com>

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

2011/3/2 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.
>
> --
> Joel
>

Hmm, not sure if it helps. But this missing -I ../../intl bugged me
already for a while.
I used the following patch in Makefile.in in gdb/common/ and I've
added it to the INCLUDES.

ChangeLog

         * common/Makfile.in (INCLUDES): Add -I for intl.

Regards,
Kai

[-- Attachment #2: unbreak_common.txt --]
[-- Type: text/plain, Size: 603 bytes --]

Index: gdb/common/Makefile.in
===================================================================
--- gdb.orig/common/Makefile.in	2011-03-02 13:20:22.000000000 +0100
+++ gdb/common/Makefile.in	2011-03-02 13:25:05.509719100 +0100
@@ -31,7 +31,7 @@ BFD_DIR = ../../bfd
 BFD_SRC = $(srcdir)/$(BFD_DIR)
 BFD_CFLAGS = -I$(BFD_DIR) -I$(BFD_SRC)
 
-INCLUDES = -I. -I../ -I$(srcdir) -I$(srcdir)/../../include @GDB_INCLUDE@
+INCLUDES = -I. -I../ -I../../intl -I$(srcdir) -I$(srcdir)/../../include @GDB_INCLUDE@
 
 libcommon_a_OBJS = signals.o $(COMMON_CPU_OBJ)
 libcommon_a_SOURCES = signals.c $(COMMON_CPU_SRC)

  reply	other threads:[~2011-03-02 12:34 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-01  6:22 Yao Qi
2011-03-02 12:14 ` Joel Brobecker
2011-03-02 12:34   ` Kai Tietz [this message]
2011-03-04  5:55     ` Joel Brobecker
2011-03-02 12:44   ` Pedro Alves
2011-03-04  5:46     ` Joel Brobecker
2011-03-02 13:12   ` Mark Kettenis
2011-03-02 14:46   ` Yao Qi
2011-03-02 15:32     ` Pedro Alves
2011-03-03  3:15       ` Yao Qi
2011-03-03  5:40         ` Yao Qi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='AANLkTi=Z1kXKyQp5wPv3eNJPg1pohyESmyJMaeSN1=Tf@mail.gmail.com' \
    --to=ktietz70@googlemail.com \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=yao@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox