[quagga-dev,16028,1/2] configure: fix static linking with readline

Message ID 9e00ae17dd32a6f36176a1f03af88606de9faadd.1471760585.git.baruch@tkos.co.il
State Under Review
Headers show
Series
  • [quagga-dev,16028,1/2] configure: fix static linking with readline
Related show

Commit Message

Baruch Siach Aug. 21, 2016, 6:23 a.m.
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

When static linking is used, the order of the libraries is important,
and the libraries using a symbol from another library should be listed
*before* the library providing that symbol (see
http://eli.thegreenplace.net/2013/07/09/library-order-in-static-linking)
for details.

When vtysh is linked statically, the command line contains "-lcurses
-lreadline", which causes a build failure due to unresolved
symbols. This is because readline is using symbols from the curses
library: the order should be the opposite.

This patch fixes that problem by putting the -lreadline at the
beginning of the LIBREADLINE variable calcualted by the configure
script.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Martin Winter Aug. 31, 2016, 7:02 p.m. | #1
Baruch,

I see this patch labeled as 1 of 2, but can't see the 2nd part (and my CI
system is ignoring it while waiting for the
2nd part.

Can you resend (both parts, preferrably labeled as v2) ?

Thanks,
   Martin Winter

On Sat, Aug 20, 2016 at 11:23 PM, Baruch Siach <baruch@tkos.co.il> wrote:

> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>
> When static linking is used, the order of the libraries is important,
> and the libraries using a symbol from another library should be listed
> *before* the library providing that symbol (see
> http://eli.thegreenplace.net/2013/07/09/library-order-in-static-linking)
> for details.
>
> When vtysh is linked statically, the command line contains "-lcurses
> -lreadline", which causes a build failure due to unresolved
> symbols. This is because readline is using symbols from the curses
> library: the order should be the opposite.
>
> This patch fixes that problem by putting the -lreadline at the
> beginning of the LIBREADLINE variable calcualted by the configure
> script.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  configure.ac | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index 6206510855c5..5cf7997a7745 100755
> --- a/configure.ac
> +++ b/configure.ac
> @@ -643,7 +643,7 @@ dnl  [TODO] on Linux, and in [TODO] on Solaris.
>               )]
>             )]
>           )
> -         AC_CHECK_LIB(readline, main, LIBREADLINE="$LIBREADLINE
> -lreadline",,
> +         AC_CHECK_LIB(readline, main, LIBREADLINE="-lreadline
> $LIBREADLINE",,
>                        "$LIBREADLINE")
>           if test $ac_cv_lib_readline_main = no; then
>             AC_MSG_ERROR([vtysh needs libreadline but was not found and
> usable on your system.])
> --
> 2.8.1
>
>
> _______________________________________________
> Quagga-dev mailing list
> Quagga-dev@lists.quagga.net
> https://lists.quagga.net/mailman/listinfo/quagga-dev
>
Baruch Siach Sept. 1, 2016, 4:22 a.m. | #2
Hi Martin,

On Wed, Aug 31, 2016 at 12:02:42PM -0700, Martin Winter wrote:
> I see this patch labeled as 1 of 2, but can't see the 2nd part (and my CI
> system is ignoring it while waiting for the
> 2nd part.

The list archive got it: 
https://lists.quagga.net/pipermail/quagga-dev/2016-August/016109.html.

> Can you resend (both parts, preferrably labeled as v2) ?

I can do that if it helps. Let me know if you need it.

baruch
Martin Winter Sept. 2, 2016, 2:39 a.m. | #3
On 31 Aug 2016, at 21:22, Baruch Siach wrote:

> Hi Martin,
>
> On Wed, Aug 31, 2016 at 12:02:42PM -0700, Martin Winter wrote:
>> I see this patch labeled as 1 of 2, but can't see the 2nd part (and 
>> my CI
>> system is ignoring it while waiting for the
>> 2nd part.
>
> The list archive got it:
> https://lists.quagga.net/pipermail/quagga-dev/2016-August/016109.html.

Not sure what went wrong, but Patchwork only got the 1st part.
See https://patchwork.quagga.net/project/quagga/list/

>> Can you resend (both parts, preferrably labeled as v2) ?
>
> I can do that if it helps. Let me know if you need it.

I would appreciate it. Or at least have the 2nd part sent again.
If the patch isn’t in Patchwork, then it won’t be tested by my CI 
system
and most likely the community will forget it (as it’s not tracked)

I’m not a patchwork expert, so sorry, don’t know what went wrong 
there.
If it still won’t make it after resending it, then let me know and 
I’ll
ping David to look into it (who maintains the patchwork)

- Martin

>      http://baruch.siach.name/blog/                  ~. .~   Tk Open 
> Systems
> =}------------------------------------------------ooO--U--Ooo------------{=
>    - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
Baruch Siach Sept. 2, 2016, 3:52 a.m. | #4
Hi Martin,

On Thu, Sep 01, 2016 at 07:39:33PM -0700, Martin Winter wrote:
> On 31 Aug 2016, at 21:22, Baruch Siach wrote:
> > On Wed, Aug 31, 2016 at 12:02:42PM -0700, Martin Winter wrote:
> > > Can you resend (both parts, preferrably labeled as v2) ?
> > 
> > I can do that if it helps. Let me know if you need it.
> 
> I would appreciate it. Or at least have the 2nd part sent again.
> If the patch isn’t in Patchwork, then it won’t be tested by my CI system
> and most likely the community will forget it (as it’s not tracked)

I sent them again. It seems like Patchwork got them both this time:

https://patchwork.quagga.net/patch/2036/
https://patchwork.quagga.net/patch/2035/

Thanks,
baruch

Patch hide | download patch | download mbox

diff --git a/configure.ac b/configure.ac
index 6206510855c5..5cf7997a7745 100755
--- a/configure.ac
+++ b/configure.ac
@@ -643,7 +643,7 @@  dnl	 [TODO] on Linux, and in [TODO] on Solaris.
 	      )]
 	    )]
 	  )
-         AC_CHECK_LIB(readline, main, LIBREADLINE="$LIBREADLINE -lreadline",,
+         AC_CHECK_LIB(readline, main, LIBREADLINE="-lreadline $LIBREADLINE",,
                       "$LIBREADLINE")
          if test $ac_cv_lib_readline_main = no; then
            AC_MSG_ERROR([vtysh needs libreadline but was not found and usable on your system.])