Re: [PATCH] configure.ac: Add missing comma to CC_LD_DYNPATH

From: Stefano Lattarini <stefano.lattarini@gmail.com>
Date: Oct 9, 2012, 21:10 UTC
Message-ID: <507492D6.3090207@gmail.com>
In-Reply-To: <1349800572-2963-1-git-send-email-sunny@sunbase.org>

[Re-sending because I forgot to CC: the list, sorry]

On 10/09/2012 06:36 PM, Øyvind A. Holm wrote:

Show 5 quoted lines

> From: "Øyvind A. Holm" <sunny@sunbase.org>
>
> 40bfbde ("build: don't duplicate substitution of make variables",
> 2012-09-11)
>

Oops, stupid copy and paste error on my part.  Sorry.

> breaks make by removing a necessary comma at the end of
> "CC_LD_DYNPATH=-rpath" in line 414 and 423.
>

Here, s/-rpath/-Wl,-rpath/, as you've noted yourself in a follow-up message. And the reference to "line 423" should be removed.

Also, as a very minor nit, I'd write "might break make" rather then "breaks make", because the breakage depends on which code path is taken at configure time (and that's why I hadn't noticed the error until now -- I never ran configure with the '--with-zlib' option).

Show 13 quoted lines

> When executing "./configure --with-zlib=PATH", this resulted in
>
>       [...]
>       CC xdiff/xhistogram.o
>       AR xdiff/lib.a
>       LINK git-credential-store
>   /usr/bin/ld: bad -rpath option
>   collect2: ld returned 1 exit status
>   make: *** [git-credential-store] Error 1
>   $
>
> during make.
>

Indeed, I can reproduce and confirm this error :-(

Show 25 quoted lines

> Signed-off-by: Øyvind A. Holm <sunny@sunbase.org>
> ---
>  configure.ac | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index da1f41f..ea79ea2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -411,7 +411,7 @@ else
>        LDFLAGS="${SAVE_LDFLAGS}"
>     ])
>     if test "$git_cv_ld_wl_rpath" = "yes"; then
> -      CC_LD_DYNPATH=-Wl,-rpath
> +      CC_LD_DYNPATH=-Wl,-rpath,
>     else
>        AC_CACHE_CHECK([if linker supports -rpath], git_cv_ld_rpath, [
>           SAVE_LDFLAGS="${LDFLAGS}"
> @@ -420,7 +420,7 @@ else
>           LDFLAGS="${SAVE_LDFLAGS}"
>        ])
>        if test "$git_cv_ld_rpath" = "yes"; then
> -         CC_LD_DYNPATH=-rpath
> +         CC_LD_DYNPATH=-rpath,
>

And as Junio noted, this second hunk is unneeded, and in fact wrong. Just remove it please.

With that done,
Acked-by: Stefano Lattarini <stefano.lattarini@gmail.com>

>        else
>           CC_LD_DYNPATH=
>           AC_MSG_WARN([linker does not support runtime path to dynamic libraries])

Thanks,
  Stefano