Include dir intl when building libcommon.a for gdb

Include dir intl when building libcommon.a for gdb

[Yao Qi]

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

On 03/01/2011 01:46 PM, Yao Qi wrote:
> 1. Build failure on mingw32, reported by Pierre. See
> <http://sourceware.org/ml/gdb-patches/2011-02/msg00285.html>
> I'll give a patch to fix this.

This patch is to fix this problem above.  Cross build gdb for
i586-mingw32msvc.  Failure goes away.

-- 
Yao (齐尧)

[-- Attachment #2: build-gdb-mingw-intl-0301.patch --]
[-- Type: text/x-patch, Size: 1355 bytes --]

gdb/
	* common/Makefile.in: Inherit CC.
	* common/configure.ac (GDB_INCLUDE): Include dir intl.
	* common/configure: Regenerate.

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@
 
 BFD_DIR = ../../bfd
 BFD_SRC = $(srcdir)/$(BFD_DIR)
diff --git a/gdb/common/configure b/gdb/common/configure
index 957dcc3..38b26ec 100755
--- a/gdb/common/configure
+++ b/gdb/common/configure
@@ -2966,7 +2966,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
 
 
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
 
 AC_SUBST(GDB_FLAGS)

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

[Joel Brobecker]

> 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

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

[Kai Tietz]

[-- 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)

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

[Pedro Alves]

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

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

[Mark Kettenis]

> 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.

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

[Yao Qi]

[-- 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])

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

[Pedro Alves]

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

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

[Yao Qi]

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 (齐尧)

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

[Yao Qi]

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 (齐尧)

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

[Joel Brobecker]

> > 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

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

[Joel Brobecker]

> 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