[PATCHEs] wafsamba: fix ordering problems with lib-provided and internal RPATHs

[Michael Adam]

Hi Metze,

this is nice, but the patch you want to revert from wafadmin is
upstream, so it would be wrong to revert it. It was also fixed
in newer waf versions. I'd also argue that your fix is not
necessarily a better fix, since the bug in question is a proper
bug in wafadmin. But it is nice that you found a way to do it in
wafsamba that is not too complicated! We can add that
additionally if you want.

And I can see whether I can use the ideas for modifying our
"wellknown libpaths" to not touch wafadmin.

Cheers - Michael

On 2014-12-18 at 20:36 +0100, Stefan (metze) Metzmacher wrote:
> Hi Michael,
> 
> here's a better fix for https://bugzilla.samba.org/show_bug.cgi?id=10548,
> which avoids modifying wafadmin.
> 
> Something like this can later also be used to remove wellknown
> library path names.
> 
> Please review and push.
> 
> Thanks!
> metze

> From 12010105b4394ae7fbc1c80e1275c7cac046e23c Mon Sep 17 00:00:00 2001
> From: Stefan Metzmacher <metze at samba.org>
> Date: Thu, 18 Dec 2014 18:09:15 +0100
> Subject: [PATCH 1/2] wafsamba: fix ordering problems with lib-provided and
>  internal RPATHs
> 
> When a library or system (like cups) provides an RPATH,
> e.g. with -Wl,-R or -Wl,-rpath, this was added by waf
> to the LINKFLAGS, wich was later prepended to our RPATH.
> But if the path by chance contains an older version of
> one of our internal libraries like talloc, this would lead
> to linking the too old talloc into our binaries.
> 
> This has been observed on, e.g., FreeBSD, but it is a general
> problem.
> 
> This patch fixes the problem by specially parsing the RPATH
> linker options from the pkg-config(, cups-config, ....) output
> and putting the paths into the RPATH_<lib> container, which
> is then later correctly appended to our internal RPATH.
> 
> This is a better fix than commit 64f5e24100a764ec198cab9a8d2c43fa86e7027c
> as it touches wafsamba only. We can revert
> 64f5e24100a764ec198cab9a8d2c43fa86e7027c in the following commit.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=10548
> 
> Signed-off-by: Stefan Metzmacher <metze at samba.org>
> ---
>  buildtools/wafsamba/samba_conftests.py | 35 ++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/buildtools/wafsamba/samba_conftests.py b/buildtools/wafsamba/samba_conftests.py
> index ec98ba0..f94b0b7 100644
> --- a/buildtools/wafsamba/samba_conftests.py
> +++ b/buildtools/wafsamba/samba_conftests.py
> @@ -4,6 +4,7 @@
>  import os, shutil, re
>  import Build, Configure, Utils
>  from Configure import conf
> +import config_c
>  from samba_utils import *
>  
>  
> @@ -506,3 +507,37 @@ def CHECK_XSLTPROC_MANPAGES(conf):
>      if not conf.CONFIG_SET('XSLTPROC_MANPAGES'):
>          print "A local copy of the docbook.xsl wasn't found on your system" \
>                " consider installing package like docbook-xsl"
> +
> +
> +waf_config_c_parse_flags = config_c.parse_flags;
> +def samba_config_c_parse_flags(line, uselib, env):
> +    waf_config_c_parse_flags(line, uselib, env)
> +
> +    try:
> +        linkflags = env['LINKFLAGS_' + uselib]
> +    except KeyError:
> +        linkflags = []
> +    for x in linkflags:
> +        #
> +        # NOTE on special treatment of -Wl,-R and -Wl,-rpath:
> +        #
> +        # It is important to not put a library provided RPATH
> +        # into the LINKFLAGS but in the RPATH instead, since
> +        # the provided LINKFLAGS get prepended to our own internal
> +        # RPATH later, and hence can potentially lead to linking
> +        # in too old versions of our internal libs.
> +        #
> +        if x.startswith('-Wl,-R,'):
> +            rpath = x[7:]
> +        elif x.startswith('-Wl,-R'):
> +            rpath = x[6:]
> +        elif x.startswith('-Wl,-rpath,'):
> +            rpath = x[11:]
> +        else:
> +            continue
> +
> +        env.append_value('RPATH_' + uselib, rpath)
> +        linkflags.remove(x)
> +
> +    return
> +config_c.parse_flags = samba_config_c_parse_flags
> -- 
> 1.9.1
> 
> 
> From 5140376e48fbf541a4dac71b3f3f3303e099112a Mon Sep 17 00:00:00 2001
> From: Stefan Metzmacher <metze at samba.org>
> Date: Thu, 18 Dec 2014 18:10:27 +0100
> Subject: [PATCH 2/2] Revert "build: fix ordering problems with lib-provided
>  and internal RPATHs"
> 
> This reverts commit 64f5e24100a764ec198cab9a8d2c43fa86e7027c.
> 
> This is not needed anymore...
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=10548
> 
> Signed-off-by: Stefan Metzmacher <metze at samba.org>
> ---
>  buildtools/wafadmin/Tools/config_c.py | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/buildtools/wafadmin/Tools/config_c.py b/buildtools/wafadmin/Tools/config_c.py
> index d0bc617..a32d8aa 100644
> --- a/buildtools/wafadmin/Tools/config_c.py
> +++ b/buildtools/wafadmin/Tools/config_c.py
> @@ -73,19 +73,6 @@ def parse_flags(line, uselib, env):
>  			app('CCFLAGS_' + uselib, x)
>  			app('CXXFLAGS_' + uselib, x)
>  			app('LINKFLAGS_' + uselib, x)
> -		#
> -		# NOTE on special treatment of -Wl,-R and -Wl,-rpath:
> -		#
> -		# It is important to not put a library provided RPATH
> -		# into the LINKFLAGS but in the RPATH instead, since
> -		# the provided LINKFLAGS get prepended to our own internal
> -		# RPATH later, and hence can potentially lead to linking
> -		# in too old versions of our internal libs.
> -		#
> -		elif x.startswith('-Wl,-R'):
> -			app('RPATH_' + uselib, x[6:])
> -		elif x.startswith('-Wl,-rpath,'):
> -			app('RPATH_' + uselib, x[11:])
>  		elif x.startswith('-Wl'):
>  			app('LINKFLAGS_' + uselib, x)
>  		elif x.startswith('-m') or x.startswith('-f'):
> -- 
> 1.9.1
> 



-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20141218/b4878595/attachment.pgp>