From: Joel Brobecker <brobecker@adacore.com>
To: Yao Qi <yao@codesourcery.com>
Cc: gdb-patches@sourceware.org
Subject: Re: Include dir intl when building libcommon.a for gdb
Date: Wed, 02 Mar 2011 12:14:00 -0000 [thread overview]
Message-ID: <20110302121407.GO30306@adacore.com> (raw)
In-Reply-To: <4D6C90AC.9010003@codesourcery.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
next prev parent reply other threads:[~2011-03-02 12:14 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 [this message]
2011-03-02 12:34 ` Kai Tietz
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=20110302121407.GO30306@adacore.com \
--to=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