Re: [patch] for mig check in GDB's configure

[陆岳 <hacklu.newborn@gmail.com>]

Hi!

thanks for your review.

On Fri, May 3, 2013 at 4:28 PM, Thomas Schwinge <thomas@codesourcery.com> wrote:
>
> As GDB is a GNU project, instead of just a commit message it uses
> ChangeLog files.  See the several ChangeLog files in the GDB sources.  As
> your change only touches files in gdb/, only gdb/ChangeLog is relevant.
> The format of the individual "snippets" is rather strict, see the
> existing ones as well as this chapter in the GNU Coding Standards:
> <http://www.gnu.org/prep/standards/html_node/Change-Logs.html>.

I have modified the ChangeLog file under gdb/ .

>
>> --- a/gdb/configure
>> +++ b/gdb/configure
>
> I take it you used autoconf to regenerate that file?

Yes! I have already removed this.

>> --- a/gdb/configure.ac
>> +++ b/gdb/configure.ac
>> @@ -488,6 +488,15 @@ AC_CHECK_TOOL(WINDRES, windres)
>>
>>  # Needed for GNU/Hurd.
>>  AC_CHECK_TOOL(MIG, mig)
>> +case "${host}" in
>
> Hmm, I think that instead of only examining the host system, $host, this
> also needs to examine the target system, $target.  (Please tell if the
> difference between build, host, and target system is not clear to you.)
> The MIG tool is used to generate files (from RPC definition files) that
> are used by the native GDB port for GNU Hurd (which, of couse, is the
> only GNU Hurd port that currently exists.)  But if someone, for example,
> builds GDB targeting mips-linux-gnu on a GNU Hurd system, they would not
> need the MIG tool.
>

To my knowledge now, $target just need to set when building a compiler
which specify which plateform your compiler generate code for. When we
build GDB, it is trivial to check the variable.
In your example, builds GDB targeting mips-linux-gnu means the GDB is
running on mips-linux. So we only need to set the
$host=mips-linux-gnu, $build=*-*-gnu.
Maybe I have got a wrong understanding about these gcc terms.

> Can you change your patch according to my review and then resend it?
> (Don't worry -- it is completely normal that patches are revised, even
> several times, before they're approved.  This helps to maintain a high
> code quality.)
>
the new one is here:

From 13d3edd1f6dbbc20b2801cea1fc367bf9042f977 Mon Sep 17 00:00:00 2001
From: hacklu <hacklu.newborn@gmail.com>
Date: Fri, 3 May 2013 18:27:08 +0800
Subject: [PATCH] Patch check mig on GNU Hurd

2013-05-3  hacklu <hacklu.newborn@gmail.com>

        * configure.ac : uncorrectly check for mig on GUN Hurd
        * configure: Regenerate.
---
 gdb/ChangeLog    |    4 ++++
 gdb/configure.ac |    9 +++++++++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 12254b7..015a878 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2013-05-3  hacklu <hacklu.newborn@gmail.com>
+
+       * configure.ac : uncorrectly check for mig on GUN Hurd
+       * configure: Regenerate.
 2013-04-30  Samuel Thibault  <samuel.thibault@gnu.org>

        * i386gnu-nat.c (CREG_OFFSET): New macro.
diff --git a/gdb/configure.ac b/gdb/configure.ac
index bb7fbdd..c1ee4cb 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -488,6 +488,15 @@ AC_CHECK_TOOL(WINDRES, windres)

 # Needed for GNU/Hurd.
 AC_CHECK_TOOL(MIG, mig)
+case "${host}" in
+       *-linux*|*-k*bsd-gnu*)
+               ;;
+       i[?]86-*-gnu*)
+           if test "$MIG" = "" ; then
+               AC_MSG_ERROR([MIG not found but required for $host])
+           fi
+           ;;
+esac

 # ---------------------- #
 # Checks for libraries.  #
--
1.7.0.4


-- 
Yue Lu (陆岳)