[PATCH] Fix target tool check logic

[PATCH] Fix target tool check logic

[Pavel Roskin]

Hello!

I promised this patch long ago, but didn't have a chance to implement  
and test it.  This patch would help users test GRUB using  
cross-compilers.  Many users would prefer to compile native GRUB  
utilities (grub-mkimage etc) but create the bootloader for another  
platform.  This is currently impossible.  To compile the bootloader  
for a foreign architecture, you have to cross-compile the GRUB  
utilities.

The current logic is broken.  The check for the target compiler is  
made if the canonical name for the build and the host are identical,  
that is if we are cross-compiling the GRUB utilities.  The matches the  
incorrect comment, but doesn't match the intention of the code.

The intention is to check for the target compiler and other tools  
after we have already found the tools for compiling for the host.

If the target has not been specified on the command line, there is no  
need to look for the target tools.  It is assumed that the host and  
the target are the same, whether we are cross-compiling or not.  The  
host tools can be used.  The target specified on the command line is  
stored in $target_alias.  Thus, if $target_alias is empty, we don't  
check for the target tools.

Further, if somebody specified the host and the target in the same  
way, there is no need to check for the target tools.  They will be the  
same.  Some package build system like to specify all possible  
switches, so they will be happy.

However, if the host and the target are specified in different ways on  
the command line, we need to check for the target tools.  That  
includes the case when the host has not been specified at all.

Even if the canonical names are the same, we should check for the  
target tools.  If $host_alias and $target_alias are different,  
configure can find different compilers.  For example, if somebody runs

./configure --host=powerpc-linux --target=powerpc-linux-uclibc

and both powerpc-linux-gcc and powerpc-linux-uclibc-gcc are present in  
$PATH, the former would be used for the host and the later for the  
target.  Generally, if the user took care to specify different values,  
we should take care not to ignore that.

ChangeLog:
         * configure.ac: Change the logic when we check for target tools.
         Do it when the target is specified and it's different from the
         specified value of the host.

--- configure.ac
+++ configure.ac
@@ -231,8 +231,8 @@
  AC_SUBST(TARGET_OBJ2ELF)
  AC_MSG_RESULT([$TARGET_OBJ2ELF])

-# For cross-compiling.
-if test "x$build" != "x$host"; then
+# Find tools for the target.
+if test -n "$target_alias" && test "x$host_alias" != "x$target_alias"; then
    # XXX this depends on the implementation of autoconf!
    tmp_ac_tool_prefix="$ac_tool_prefix"
    ac_tool_prefix=$target_alias-

-- 
Regards,
Pavel Roskin

Re: [PATCH] Fix target tool check logic

[Yoshinori K. Okuji]

On Saturday 11 April 2009 13:58:26 Pavel Roskin wrote:
> Hello!
>
> I promised this patch long ago, but didn't have a chance to implement
> and test it.  This patch would help users test GRUB using
> cross-compilers.  Many users would prefer to compile native GRUB
> utilities (grub-mkimage etc) but create the bootloader for another
> platform.  This is currently impossible.  To compile the bootloader
> for a foreign architecture, you have to cross-compile the GRUB
> utilities.
>
> The current logic is broken.  The check for the target compiler is
> made if the canonical name for the build and the host are identical,
> that is if we are cross-compiling the GRUB utilities.  The matches the
> incorrect comment, but doesn't match the intention of the code.
>
> The intention is to check for the target compiler and other tools
> after we have already found the tools for compiling for the host.
>
> If the target has not been specified on the command line, there is no
> need to look for the target tools.  It is assumed that the host and
> the target are the same, whether we are cross-compiling or not.  The
> host tools can be used.  The target specified on the command line is
> stored in $target_alias.  Thus, if $target_alias is empty, we don't
> check for the target tools.
>
> Further, if somebody specified the host and the target in the same
> way, there is no need to check for the target tools.  They will be the
> same.  Some package build system like to specify all possible
> switches, so they will be happy.
>
> However, if the host and the target are specified in different ways on
> the command line, we need to check for the target tools.  That
> includes the case when the host has not been specified at all.
>
> Even if the canonical names are the same, we should check for the
> target tools.  If $host_alias and $target_alias are different,
> configure can find different compilers.  For example, if somebody runs
>
> ./configure --host=powerpc-linux --target=powerpc-linux-uclibc
>
> and both powerpc-linux-gcc and powerpc-linux-uclibc-gcc are present in
> $PATH, the former would be used for the host and the later for the
> target.  Generally, if the user took care to specify different values,
> we should take care not to ignore that.
>
> ChangeLog:
>          * configure.ac: Change the logic when we check for target tools.
>          Do it when the target is specified and it's different from the
>          specified value of the host.
>
> --- configure.ac
> +++ configure.ac
> @@ -231,8 +231,8 @@
>   AC_SUBST(TARGET_OBJ2ELF)
>   AC_MSG_RESULT([$TARGET_OBJ2ELF])
>
> -# For cross-compiling.
> -if test "x$build" != "x$host"; then
> +# Find tools for the target.
> +if test -n "$target_alias" && test "x$host_alias" != "x$target_alias";

"test -n" should be avoided. Maybe this is not necessary nowadays, but my old 
lesson was to use "test x$target_alias != x" instead for portability. 
Well, "!=" was not very portable, either, maybe.

> then # XXX this depends on the implementation of autoconf!
>     tmp_ac_tool_prefix="$ac_tool_prefix"
>     ac_tool_prefix=$target_alias-


Regards,
Okuji

Re: [PATCH] Fix target tool check logic

[Pavel Roskin]

Quoting "Yoshinori K. Okuji" <okuji@enbug.org>:

> "test -n" should be avoided. Maybe this is not necessary nowadays, but my old
> lesson was to use "test x$target_alias != x" instead for portability.
> Well, "!=" was not very portable, either, maybe.

I believe both "-n" and "!=" are found in Autoconf sources that are  
turned into  configure scripts.  Anyway, I'll use the syntax you want.

-- 
Regards,
Pavel Roskin

Re: [PATCH] Fix target tool check logic

[Yoshinori K. Okuji]

On Saturday 11 April 2009 22:16:58 Pavel Roskin wrote:
> Quoting "Yoshinori K. Okuji" <okuji@enbug.org>:
> > "test -n" should be avoided. Maybe this is not necessary nowadays, but my
> > old lesson was to use "test x$target_alias != x" instead for portability.
> > Well, "!=" was not very portable, either, maybe.
>
> I believe both "-n" and "!=" are found in Autoconf sources that are
> turned into  configure scripts.  Anyway, I'll use the syntax you want.

Even if this looks obsolete, I think it is better to follow the 
chapter "Limitations of Builtins" in the autoconf manual:

`test' (strings)
     Posix says that `test "STRING"' succeeds if STRING is not null,
     but this usage is not portable to traditional platforms like
     Solaris 10 `/bin/sh', which mishandle strings like `!' and `-n'.

     Posix also says that `test ! "STRING"', `test -n "STRING"' and
     `test -z "STRING"' work with any string, but many shells (such as
     Solaris, AIX 3.2, UNICOS 10.0.0.6, Digital Unix 4, etc.) get
     confused if STRING looks like an operator:

          $ test -n =
          test: argument expected
          $ test ! -n
          test: argument expected

     Similarly, Posix says that both `test "STRING1" = "STRING2"' and
     `test "STRING1" != "STRING2"' work for any pairs of strings, but
     in practice this is not true for troublesome strings that look
     like operators or parentheses, or that begin with `-'.

     It is best to protect such strings with a leading `X', e.g., `test
     "XSTRING" != X' rather than `test -n "STRING"' or `test !
     "STRING"'.

Regards,
Okuji

Re: [PATCH] Fix target tool check logic

[Pavel Roskin]

On Wed, 2009-04-15 at 00:45 +0900, Yoshinori K. Okuji wrote:
> On Saturday 11 April 2009 22:16:58 Pavel Roskin wrote:
> > Quoting "Yoshinori K. Okuji" <okuji@enbug.org>:
> > > "test -n" should be avoided. Maybe this is not necessary nowadays, but my
> > > old lesson was to use "test x$target_alias != x" instead for portability.
> > > Well, "!=" was not very portable, either, maybe.
> >
> > I believe both "-n" and "!=" are found in Autoconf sources that are
> > turned into  configure scripts.  Anyway, I'll use the syntax you want.
> 
> Even if this looks obsolete, I think it is better to follow the 
> chapter "Limitations of Builtins" in the autoconf manual:

Thanks.  The Autoconf code I was referring to didn't involve any
possibility of pathological arguments.  But we are dealing with user
input here (target_alias comes from the command line), so you are right,
it's better to err on the safe side.

-- 
Regards,
Pavel Roskin