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

[Thomas Schwinge <thomas@codesourcery.com>]

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

Hi!

Adding the gdb-patches mailing list.  While of course this is only
relevant for the GNU Hurd port of GDB, it will get committed to the GDB
source repository, so should be reviewed on the gdb-patches mailing list.
It is fine (and encouraged) to CC the bug-hurd mailing list for
Hurd-specific issues, though.


For the GDB folks, Yue Lu is a candidate for improving the GNU Hurd GDB
port as a Google Summer of Code 2013 project, and is sending here a first
patch.  Welcome to the project!


On Fri, 3 May 2013 15:15:39 +0800, 陆岳 <hacklu.newborn@gmail.com> wrote:
> I found that when you missing the mid under GNU Hurd, the GDB's
> configure doesn't complain about that.
> But you will get a compile error until you do the make.
> So I add the check.
> By the way, I just check the existence of mig, have not check whether
> mig work correct yet.
> 
> This is my first time to submit patch, I just build this by git
> format-patch. If something wrong, just tell me, I will fix it.

I acknowledge the issue: as $(MIG) will be empty, you'll get the command
line »gcc [...] |  -cc [...]« resulting in a confusing »-cc: command not
found«.  So, thanks for the patch!  Given this is your first patch, it
looks very good already!


> Subject: [PATCH] patch for check mig under GNU Hurd
> 
>         if no mig for use then exit!

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

What developers typically do when committing their changes is re-using
the ChangeLog snippet as the commit message.  Commits are still done with
CVS, by the way -- the Git repository is just a read-only mirror.  Until
you're approved for getting commit access yourself, another developer
will commit any approved patches for you.  (I can do that then.)


> --- a/gdb/configure
> +++ b/gdb/configure

I take it you used autoconf to regenerate that file?


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

GDB folks, would it make sense to use something like:

    case $gdb_native:$host in
      [...]
      yes:i[[3456]]86-*-gnu*)
        [error if MIG not found]

..., to check that both host and target are GNU Hurd?

> +       *-linux*|*-k*bsd-gnu*)
> +               ;;

Very right: these triples need to be special-cased first, as the
following *-gnu* will also match i686-pc-linux-gnu, for example.

> +       i[[3456789]]86-*-gnu*)

Typically, only i[3456]86 are used, I think.  Or, just use i?86.

> +           if test "$MIG" = "" ; then
> +               AC_MSG_ERROR([no mig for use])

I'd say something like »MIG not found but required for $host«.

> +           fi
> +           ;;
> +esac
> 
>  # ---------------------- #
>  # Checks for libraries.  #


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


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]