This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: PATCH: PR gold/14897: gold is installed as default ld by accident
- From: Ian Lance Taylor <iant at google dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: binutils at sourceware dot org
- Date: Mon, 7 Jan 2013 10:41:44 -0800
- Subject: Re: PATCH: PR gold/14897: gold is installed as default ld by accident
- References: <20121130150235.GA7366@intel.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