Re: PATCH: PR gold/14897: gold is installed as default ld by accident

[Ian Lance Taylor <iant at google dot com>]

On Fri, Nov 30, 2012 at 7:02 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>
> We should install gold as default ld only for --disable-ld or
> --enable-gold=default.  Tested with
>
> --enable-gold
> --enable-gold=default
> --enable-gold --disable-ld
>
> OK for trunk and 2.23?
>
> Thanks.
>
> H.J.
> ---
> 2012-11-30  H.J. Lu  <hongjiu.lu@intel.com>
>
>         PR gold/14897
>         * configure.ac (install_as_default): Set to yes only for
>         --disable-ld or --enable-gold=default.
>         * configure: Regenerated.
> diff --git a/gold/configure b/gold/configure
> index 4f74ae3..16737ae 100755
> --- a/gold/configure
> +++ b/gold/configure
> @@ -3272,18 +3272,22 @@ default_ld=
>  # Check whether --enable-ld was given.
>  if test "${enable_ld+set}" = set; then :
>    enableval=$enable_ld; case "${enableval}" in
> -  default)
> -    default_ld=ld.bfd
> -    ;;
> -esac
> + no)
> +   default_ld=ld.gold
> +   ;;
> + esac
>  fi
>
>
>  # Check whether --enable-gold was given.
>  if test "${enable_gold+set}" = set; then :
>    enableval=$enable_gold; case "${enableval}" in
> - yes|default)
> -   if test x${default_ld} = x; then
> + default)
> +   install_as_default=yes
> +   installed_linker=ld.gold
> +   ;;
> + yes)
> +   if test x${default_ld} != x; then
>       install_as_default=yes
>     fi
>     installed_linker=ld.gold
> diff --git a/gold/configure.ac b/gold/configure.ac
> index f8297f2..cbb2326 100644
> --- a/gold/configure.ac
> +++ b/gold/configure.ac
> @@ -55,16 +55,20 @@ default_ld=
>  AC_ARG_ENABLE(ld,
>  [[  --enable-ld[=ARG]     build ld [ARG={default,yes,no}]]],
>  [case "${enableval}" in
> -  default)
> -    default_ld=ld.bfd
> -    ;;
> -esac])
> + no)
> +   default_ld=ld.gold
> +   ;;
> + esac])
>
>  AC_ARG_ENABLE(gold,
>  [[  --enable-gold[=ARG]     build gold [ARG={default,yes,no}]]],
>  [case "${enableval}" in
> - yes|default)
> -   if test x${default_ld} = x; then
> + default)
> +   install_as_default=yes
> +   installed_linker=ld.gold
> +   ;;
> + yes)
> +   if test x${default_ld} != x; then
>       install_as_default=yes
>     fi
>     installed_linker=ld.gold

I find the logic of this patch fairly hard to puzzle out.

I think it would be easier to understand if you wrote it more like this:

AC_ARG_ENABLE(ld, [[  --enable-ld[=ARG]     build ld [ARG={default,yes,no}]]])

installed_linker=ld.gold
AC_ARG_ENABLE(gold,
[[  --enable-gold[=ARG]     build gold [ARG={default,yes,no}]]],
[case "${enableval}" in
 default)
   install_as_default=yes
   ;;
 yes)
   if test x${enable_ld} = xno; then
    install_as_default=yes
  fi
  ;;
esac])

I think the logic here is clearer: we set install_as_default when we
see --enable-gold=default, or when we see --enable-gold --disable-ld.

That is an example without changing gold/Makefile.am.  But really
there is no reason for the installed_linker variable.  The only value
that installed_linker ever gets is ld.gold; that is, in the current
regime, gold/Makefile.am can always install gold as ld.gold.  The only
interesting option is whether to also install gold as ld.  So why not
simplify?  We don't need to preserve the history of how we got here,
we want to have something easy to understand when you look at it.

Ian