public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* PATCH: PR gold/14897: gold is installed as default ld by accident
@ 2012-11-30 15:02 H.J. Lu
  2013-01-07 17:06 ` H.J. Lu
  2013-01-07 18:41 ` Ian Lance Taylor
  0 siblings, 2 replies; 5+ messages in thread
From: H.J. Lu @ 2012-11-30 15:02 UTC (permalink / raw)
  To: binutils; +Cc: Ian Lance Taylor

Hi,

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: PATCH: PR gold/14897: gold is installed as default ld by accident
  2012-11-30 15:02 PATCH: PR gold/14897: gold is installed as default ld by accident H.J. Lu
@ 2013-01-07 17:06 ` H.J. Lu
  2013-01-07 18:41 ` Ian Lance Taylor
  1 sibling, 0 replies; 5+ messages in thread
From: H.J. Lu @ 2013-01-07 17:06 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: binutils

On Fri, Nov 30, 2012 at 7:02 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> Hi,
>
> 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.

> --- 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, Ian Lance Taylor <iant@google.com>
> +   ;;
> + yes)
> +   if test x${default_ld} != x; then
>       install_as_default=yes
>     fi
>     installed_linker=ld.gold

Hi Ian,

Can you take a look at this patch?  This problem shows up when
running "make install" under the gold directory.   It isn't a problem
when running "make install" under the toplevel directory since
install-ld depends on install-gold.

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: PATCH: PR gold/14897: gold is installed as default ld by accident
  2012-11-30 15:02 PATCH: PR gold/14897: gold is installed as default ld by accident H.J. Lu
  2013-01-07 17:06 ` H.J. Lu
@ 2013-01-07 18:41 ` Ian Lance Taylor
  2013-01-07 19:32   ` H.J. Lu
  1 sibling, 1 reply; 5+ messages in thread
From: Ian Lance Taylor @ 2013-01-07 18:41 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: PATCH: PR gold/14897: gold is installed as default ld by accident
  2013-01-07 18:41 ` Ian Lance Taylor
@ 2013-01-07 19:32   ` H.J. Lu
  2013-01-07 19:48     ` Ian Lance Taylor
  0 siblings, 1 reply; 5+ messages in thread
From: H.J. Lu @ 2013-01-07 19:32 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: binutils

On Mon, Jan 7, 2013 at 10:41 AM, Ian Lance Taylor <iant@google.com> wrote:
>
> 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.
>

There is no need for

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

since we only check

if test x${enable_ld} = xno; then

enable_ld is set to no for --disable-ld by default.  Here is the
updated patch.  OK for trunk?

Thanks.


-- 
H.J.
---
2013-01-07  H.J. Lu  <hongjiu.lu@intel.com>
	    Ian Lance Taylor  <iant@google.com>

	PR gold/14897
	* configure.ac (--enable-ld): Removed.
	(install_as_default): Set to yes only for --enable-gold=default
	or --disable-ld.
	* configure: Regenerated.

diff --git a/gold/configure.ac b/gold/configure.ac
index e49d6e8..804a474 100644
--- a/gold/configure.ac
+++ b/gold/configure.ac
@@ -68,32 +68,20 @@ dnl "install_as_default" is true if the linker to be install
ed as the
 dnl default linker, ld.
 dnl "installed_linker" is the installed gold linker name.

-default_ld=
-AC_ARG_ENABLE(ld,
-[[  --enable-ld[=ARG]     build ld [ARG={default,yes,no}]]],
-[case "${enableval}" in
-  default)
-    default_ld=ld.bfd
-    ;;
-esac])
-
+installed_linker=ld.gold
 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
+   ;;
+ yes)
+   if test x${enable_ld} = xno; then
      install_as_default=yes
    fi
-   installed_linker=ld.gold
-   ;;
- no)
-   ;;
- *)
-   AC_MSG_ERROR([invalid --enable-gold argument])
    ;;
  esac],
-[install_as_default=no
- installed_linker=ld.gold])
+[install_as_default=no])
 AC_SUBST(install_as_default)
 AC_SUBST(installed_linker)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: PATCH: PR gold/14897: gold is installed as default ld by accident
  2013-01-07 19:32   ` H.J. Lu
@ 2013-01-07 19:48     ` Ian Lance Taylor
  0 siblings, 0 replies; 5+ messages in thread
From: Ian Lance Taylor @ 2013-01-07 19:48 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Mon, Jan 7, 2013 at 11:32 AM, H.J. Lu <hjl.tools@gmail.com> wrote:

> 2013-01-07  H.J. Lu  <hongjiu.lu@intel.com>
>             Ian Lance Taylor  <iant@google.com>
>
>         PR gold/14897
>         * configure.ac (--enable-ld): Removed.
>         (install_as_default): Set to yes only for --enable-gold=default
>         or --disable-ld.
>         * configure: Regenerated.

This is OK.

Thanks.

Ian

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-01-07 19:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-30 15:02 PATCH: PR gold/14897: gold is installed as default ld by accident H.J. Lu
2013-01-07 17:06 ` H.J. Lu
2013-01-07 18:41 ` Ian Lance Taylor
2013-01-07 19:32   ` H.J. Lu
2013-01-07 19:48     ` Ian Lance Taylor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).