* Re: Include dir intl when building libcommon.a for gdb
2011-03-02 12:14 ` Joel Brobecker
@ 2011-03-02 12:34 ` Kai Tietz
2011-03-04 5:55 ` Joel Brobecker
2011-03-02 12:44 ` Pedro Alves
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Kai Tietz @ 2011-03-02 12:34 UTC (permalink / raw)
To: Joel Brobecker; +Cc: Yao Qi, gdb-patches
[-- 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)
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: Include dir intl when building libcommon.a for gdb
2011-03-02 12:34 ` Kai Tietz
@ 2011-03-04 5:55 ` Joel Brobecker
0 siblings, 0 replies; 11+ messages in thread
From: Joel Brobecker @ 2011-03-04 5:55 UTC (permalink / raw)
To: Kai Tietz; +Cc: Yao Qi, gdb-patches
> 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.
Could you give the initial error message? Was it when building
GDB or GDBserver?
I am going to guess that this was when you were building GDB?
GDBserver does not seem to include intl, as far as I can tell, so
it would suggest, as pointed out by Pedro, that this should be
put in GDB_INCLUDE instead?
But this also suggests that we have some other code that is in
common/ and should not be there. Your patch may fix the symptoms,
but we'll need to look at the root cause as well.
> 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)
--
Joel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Include dir intl when building libcommon.a for gdb
2011-03-02 12:14 ` Joel Brobecker
2011-03-02 12:34 ` Kai Tietz
@ 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
3 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2011-03-02 12:44 UTC (permalink / raw)
To: gdb-patches; +Cc: Joel Brobecker, Yao Qi
On Wednesday 02 March 2011 12:14:07, Joel Brobecker wrote:
> 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.
Thank you. +1. I said something like that from the beginning,
but I didn't imagine that a configure&make under common/ sooner
than later would actually make our lifes complicated. I have
since learned, and I believe it was a wrong step to make.
I'd much rather we were spending energy on that foundation rather
than on the build, when we have only a handful of files to share
at this time.
> 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.
gdbserver does not depend on bfd. It's wrong to leave it in
the include path. gdbserver is not using gnulib either (memmem.o hack
doesn't count), so there goes another include path that
should not be present on gdbserver's common/ build, and should be on
gdb's common/ build, but isn't at present, I think. That's a
bug, and it will show its face as soon as something under
common/ needs to include a gnulib header on some host (it may
already, haven't checked, linux hosts doesn't need any at present).
--
Pedro Alves
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Include dir intl when building libcommon.a for gdb
2011-03-02 12:44 ` Pedro Alves
@ 2011-03-04 5:46 ` Joel Brobecker
0 siblings, 0 replies; 11+ messages in thread
From: Joel Brobecker @ 2011-03-04 5:46 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Yao Qi
> > 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.
>
> gdbserver does not depend on bfd. It's wrong to leave it in
> the include path. gdbserver is not using gnulib either (memmem.o hack
> doesn't count),
Coming back to this, this makes me ask myself: If GDBserver does
not use bfd, for instance, and we have code in common/ that needs
bfd, does it not mean that we have code in common/ that is, well,
not common between GDB and GDBserver??? IMO, we do not want that.
--
Joel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Include dir intl when building libcommon.a for gdb
2011-03-02 12:14 ` Joel Brobecker
2011-03-02 12:34 ` Kai Tietz
2011-03-02 12:44 ` Pedro Alves
@ 2011-03-02 13:12 ` Mark Kettenis
2011-03-02 14:46 ` Yao Qi
3 siblings, 0 replies; 11+ messages in thread
From: Mark Kettenis @ 2011-03-02 13:12 UTC (permalink / raw)
To: brobecker; +Cc: yao, gdb-patches
> 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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Include dir intl when building libcommon.a for gdb
2011-03-02 12:14 ` Joel Brobecker
` (2 preceding siblings ...)
2011-03-02 13:12 ` Mark Kettenis
@ 2011-03-02 14:46 ` Yao Qi
2011-03-02 15:32 ` Pedro Alves
3 siblings, 1 reply; 11+ messages in thread
From: Yao Qi @ 2011-03-02 14:46 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1935 bytes --]
On 03/02/2011 08:14 PM, Joel Brobecker wrote:
>
> 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").
To your first question, because common/signals.c uses "ifdef GDBSERVER",
which makes a lot troubles here. I think of this problem again, and
find that root cause of this problem is that files in common dir
includes some gdb-specific or gdbserver specific code.
If we move gdb-specific part of common/signals.c out of it, the include
and compilation flags can be unique.
This patch is to move gdb-specific code out of common/signals.c.
http://sourceware.org/ml/gdb-patches/2011-02/msg00669.html
Once this patch above is applied, we can remove these conditional
includes and flags setting code.
> 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 :-(.
>
Yes, we should keep libcommon.a compilation flags the same. As I said
above, we can make it independent via moving some of codes out of signals.c.
--
Yao (é½å°§)
[-- Attachment #2: common-conf-make-0302.patch --]
[-- Type: text/x-patch, Size: 3251 bytes --]
gdb/
* common/Makefile.in: Remove BFD_DIR, BFD_SRC, BFD_CFLAGS.
(INCLUDES): Remove GDB_INCLUDE.
(COMPILE): Remove GDB_FLAGS.
* common/configure.ac: Remove conditionals to set includes
and compilation flags.
* common/configure: Regenerate.
diff --git a/gdb/common/Makefile.in b/gdb/common/Makefile.in
index 9230b87..6782631 100644
--- a/gdb/common/Makefile.in
+++ b/gdb/common/Makefile.in
@@ -27,11 +27,7 @@ COMMON_CPU_OBJ = @COMMON_CPU_OBJ@
# when running make. I.E. "make CFLAGS=-Wmissing-prototypes".
CFLAGS = @CFLAGS@
-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$(srcdir) -I$(srcdir)/../../include
libcommon_a_OBJS = signals.o $(COMMON_CPU_OBJ)
libcommon_a_SOURCES = signals.c $(COMMON_CPU_SRC)
@@ -42,7 +38,7 @@ signals.o: $(srcdir)/signals.c
$(COMPILE) $(srcdir)/signals.c
COMPILE = source='$<' object='$@' libtool=no $(CC) $(DEFS) $(INCLUDES) \
- @GDB_FLAGS@ $(CPPFLAGS) $(ALL_CFLAGS) $(CFLAGS) -c
+ $(CPPFLAGS) $(ALL_CFLAGS) $(CFLAGS) -c
# Implicit rules
diff --git a/gdb/common/configure b/gdb/common/configure
index 957dcc3..31b353e 100755
--- a/gdb/common/configure
+++ b/gdb/common/configure
@@ -553,8 +553,6 @@ PACKAGE_URL=
ac_unique_file="."
ac_subst_vars='LTLIBOBJS
LIBOBJS
-GDB_INCLUDE
-GDB_FLAGS
COMMON_CPU_SRC
COMMON_CPU_OBJ
SET_MAKE
@@ -625,7 +623,6 @@ SHELL'
ac_subst_files=''
ac_user_opts='
enable_option_checking
-enable_gdbserver
'
ac_precious_vars='build_alias
host_alias
@@ -1244,12 +1241,6 @@ if test -n "$ac_init_help"; then
cat <<\_ACEOF
-Optional Features:
- --disable-option-checking ignore unrecognized --enable/--with options
- --disable-FEATURE do not include FEATURE (same as --enable-FEATURE=no)
- --enable-FEATURE[=ARG] include FEATURE [ARG=yes]
- --enable-gdbserver build libcommon.a for gdbserver
-
Some influential environment variables:
CC C compiler command
CFLAGS C compiler flags
@@ -2952,26 +2943,6 @@ case "$host_cpu" in
;;
esac
-# Determine whether or not build libcommon.a for gdbserver
-# Check whether --enable-gdbserver was given.
-if test "${enable_gdbserver+set}" = set; then :
- enableval=$enable_gdbserver;
-else
- enable_gdbserver=no
-fi
-
-
-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
-
-
-
-
ac_config_files="$ac_config_files Makefile"
diff --git a/gdb/common/configure.ac b/gdb/common/configure.ac
index 1ef85fe..4f0d904 100644
--- a/gdb/common/configure.ac
+++ b/gdb/common/configure.ac
@@ -53,22 +53,6 @@ case "$host_cpu" in
;;
esac
-# Determine whether or not build libcommon.a for gdbserver
-AC_ARG_ENABLE(gdbserver,
-[ --enable-gdbserver build libcommon.a for gdbserver],
-[], [enable_gdbserver=no])
-
-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
-
-AC_SUBST(GDB_FLAGS)
-AC_SUBST(GDB_INCLUDE)
-
AC_EXEEXT
AC_OUTPUT([Makefile])
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: Include dir intl when building libcommon.a for gdb
2011-03-02 14:46 ` Yao Qi
@ 2011-03-02 15:32 ` Pedro Alves
2011-03-03 3:15 ` Yao Qi
0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2011-03-02 15:32 UTC (permalink / raw)
To: gdb-patches; +Cc: Yao Qi, Joel Brobecker
On Wednesday 02 March 2011 14:46:44, Yao Qi wrote:
> To your first question, because common/signals.c uses "ifdef GDBSERVER",
> which makes a lot troubles here. I think of this problem again, and
> find that root cause of this problem is that files in common dir
> includes some gdb-specific or gdbserver specific code.
You're oversimplifying. You'll need to include one or the
other in files under common/ that require types such as CORE_ADDR
and others. Obvious solutions to that are to rename server.h to
defs.h, or add a new defs.h that just includes server.h.
> If we move gdb-specific part of common/signals.c out of it, the include
> and compilation flags can be unique.
Only because signals.c doesn't use any of gdb/gdbserver's specific
typedefs (CORE_ADDR & co).
I haven't seen any comment on AC_CHECK_HEADERS concerns
I raised, probably because the FSF tree has only signals.c
and not much else under common/ currently, which hides
the issue.
Grepping the code we have under common/ in our local tree
for "HAVE_", I see:
$ grep HAVE_ *
gdb_dirent.h:#ifdef HAVE_DIRENT_H
gdb_dirent.h:# ifdef HAVE_SYS_NDIR_H
gdb_dirent.h:# ifdef HAVE_SYS_DIR_H
gdb_dirent.h:# ifdef HAVE_NDIR_H
gdb_locale.h:#ifdef HAVE_LOCALE_H
gdb_locale.h:#ifdef HAVE_LANGINFO_CODESET
signals.c:#ifdef HAVE_SIGNAL_H
Maintaining and making sure these checks are in both configures,
rather than centralized is harder than updating two Makefiles.
IMO, this should be handled by an .m4 under common that is
used by both configures, something like gnulib/'s gl_INIT.
gnulib does not have its own configure, yet it integrates into
random configures. It looks like a better model for common/
to me.
--
Pedro Alves
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Include dir intl when building libcommon.a for gdb
2011-03-02 15:32 ` Pedro Alves
@ 2011-03-03 3:15 ` Yao Qi
2011-03-03 5:40 ` Yao Qi
0 siblings, 1 reply; 11+ messages in thread
From: Yao Qi @ 2011-03-03 3:15 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Joel Brobecker
On 03/02/2011 11:32 PM, Pedro Alves wrote:
> On Wednesday 02 March 2011 14:46:44, Yao Qi wrote:
>> To your first question, because common/signals.c uses "ifdef GDBSERVER",
>> which makes a lot troubles here. I think of this problem again, and
>> find that root cause of this problem is that files in common dir
>> includes some gdb-specific or gdbserver specific code.
>
> You're oversimplifying. You'll need to include one or the
> other in files under common/ that require types such as CORE_ADDR
> and others. Obvious solutions to that are to rename server.h to
> defs.h, or add a new defs.h that just includes server.h.
>
I don't understand why it is "oversimplying". We should make code in
common/ dir as "common" as possible, and don't need conditional includes
and flags.
This approach meets needs so far, since there is only one c file in
common dir, and doesn't require types such CORE_ADDR. When we moving
more and more files to common/, we should move some common types, like
CORE_ADDR, into common as well.
> I haven't seen any comment on AC_CHECK_HEADERS concerns
> I raised, probably because the FSF tree has only signals.c
> and not much else under common/ currently, which hides
> the issue.
>
> Grepping the code we have under common/ in our local tree
> for "HAVE_", I see:
>
> $ grep HAVE_ *
> gdb_dirent.h:#ifdef HAVE_DIRENT_H
> gdb_dirent.h:# ifdef HAVE_SYS_NDIR_H
> gdb_dirent.h:# ifdef HAVE_SYS_DIR_H
> gdb_dirent.h:# ifdef HAVE_NDIR_H
> gdb_locale.h:#ifdef HAVE_LOCALE_H
> gdb_locale.h:#ifdef HAVE_LANGINFO_CODESET
> signals.c:#ifdef HAVE_SIGNAL_H
>
> Maintaining and making sure these checks are in both configures,
> rather than centralized is harder than updating two Makefiles.
> IMO, this should be handled by an .m4 under common that is
> used by both configures, something like gnulib/'s gl_INIT.
Yes, .m4 file can reduce duplication of configures in both gdb and
gdbserver. I agree.
> gnulib does not have its own configure, yet it integrates into
> random configures. It looks like a better model for common/
> to me.
We can have configure/make and .m4 together in common/. .m4 file
provides common macros, while configure generates Makefile for source in
common/.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Include dir intl when building libcommon.a for gdb
2011-03-03 3:15 ` Yao Qi
@ 2011-03-03 5:40 ` Yao Qi
0 siblings, 0 replies; 11+ messages in thread
From: Yao Qi @ 2011-03-03 5:40 UTC (permalink / raw)
To: gdb-patches
On 03/03/2011 11:15 AM, Yao Qi wrote:
>> > gnulib does not have its own configure, yet it integrates into
>> > random configures. It looks like a better model for common/
>> > to me.
> We can have configure/make and .m4 together in common/. .m4 file
> provides common macros, while configure generates Makefile for source in
> common/.
Think it over again during lunch today, if there is no extra checking in
common/configure.ac, then we can remove it, and generate common/Makefile
in gdb/configure.ac and gdbserver/configure.ac.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 11+ messages in thread