Meson produces incorrect rpath for local libraries #31222

jtojnar · 2017-11-04T01:12:41Z

Issue description

Dejà dup requires a custom library in $ORIGIN/../lib/deja-dup but meson, for some reason, adds $ORIGIN/../libdeja to the rpath:

$ chrpath --list result/bin/.deja-dup-wrapped 
result/bin/.deja-dup-wrapped: RUNPATH=$ORIGIN/../libdeja:$ORIGIN/widgets:XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX:/nix/store/d9s2m4fs3m8ifngnkzmpik6li1yqy0z9-glib-2.54.1/lib:/nix/store/hn0h1dfhap9dm69ns6fjqicip05ripkh-gtk+3-3.22.24/lib:/nix/store/m4bj2aya32k8jj93a81acj6ixwp82lh5-pango-1.40.12/lib:/nix/store/4pbw7113798fd5n7n1azfql3d6mcs7cm-atk-2.26.0/lib:/nix/store/jj2agpnlyv1km4p7d7gqh1z4k3lbjfsa-libsecret-0.18.5/lib:/nix/store/03v97inn212jf1lychvsz2if6f1n4wzb-glibc-2.25-49/lib

If I remove the postPatch attribute, the path to the custom library is in rpath but then we do not have the global ones:

$ chrpath --list result/bin/.deja-dup-wrapped 
result/bin/.deja-dup-wrapped: RUNPATH=/nix/store/xb23lkk89hdh6g160daankb7cvspw49s-deja-dup-36.3/lib/deja-dup

Steps to reproduce

git fetch upstream pull/31219/head:deja-dup-rpath
git checkout deja-dup-rpath
nix-build -A deja-dup
chrpath --list result/bin/.deja-dup-wrapped

Technical details

System: 18.03.git.27e00b4 (Impala)
Nix version: 1.11.15
Nixpkgs version: 5cfd049
Sandboxing enabled: true

The text was updated successfully, but these errors were encountered:

Some apps require an internal library, that needs to be added to rpath. In 8f95aef, we removed the rpath fixup, breaking these apps. This commit re-adds the fixup but modifies it to keep the old rpath. Closes: NixOS#31222

jtojnar · 2017-11-11T08:46:02Z

@yegortimoshenko I found where those X’s come from. Could we maybe do something like this instead?

diff --git a/mesonbuild/compilers/compilers.py b/mesonbuild/compilers/compilers.py
index 3f088b0f..a2661240 100644
--- a/mesonbuild/compilers/compilers.py
+++ b/mesonbuild/compilers/compilers.py
@@ -834,12 +834,11 @@ class Compiler:
         # Build_rpath is used as-is (it is usually absolute).
         if build_rpath != '':
             paths += ':' + build_rpath
-        if len(paths) < len(install_rpath):
-            padding = 'X' * (len(install_rpath) - len(paths))
-            if not paths:
-                paths = padding
-            else:
-                paths = paths + ':' + padding
+        padding = 'X' * len(install_rpath)
+        if not paths:
+            paths = padding
+        else:
+            paths = paths + ':' + padding
         args = ['-Wl,-rpath,' + paths]
         if get_compiler_is_linuxlike(self):
             # Rpaths to use while linking must be absolute. These are not
diff --git a/mesonbuild/scripts/depfixer.py b/mesonbuild/scripts/depfixer.py
index ee63147d..85f81506 100644
--- a/mesonbuild/scripts/depfixer.py
+++ b/mesonbuild/scripts/depfixer.py
@@ -300,19 +300,8 @@ class Elf(DataSizes):
             return
         self.bf.seek(rp_off)
         old_rpath = self.read_str()
-        if len(old_rpath) < len(new_rpath):
-            sys.exit("New rpath must not be longer than the old one.")
-        # The linker does read-only string deduplication. If there is a
-        # string that shares a suffix with the rpath, they might get
-        # dedupped. This means changing the rpath string might break something
-        # completely unrelated. This has already happened once with X.org.
-        # Thus we want to keep this change as small as possible to minimize
-        # the chance of obliterating other strings. It might still happen
-        # but our behavior is identical to what chrpath does and it has
-        # been in use for ages so based on that this should be rare.
-        if not new_rpath:
-            self.remove_rpath_entry(entrynum)
-        else:
+        if new_rpath:
+            new_rpath = old_rpath.replace('X' * len(new_rpath), new_rpath)
             self.bf.seek(rp_off)
             self.bf.write(new_rpath)
             self.bf.write(b'\0')

jtojnar · 2017-11-11T08:55:04Z

Or even better, skip the X nonsense:

diff --git a/mesonbuild/compilers/compilers.py b/mesonbuild/compilers/compilers.py
index 3f088b0f..71a5fe8e 100644
--- a/mesonbuild/compilers/compilers.py
+++ b/mesonbuild/compilers/compilers.py
@@ -834,12 +834,10 @@ class Compiler:
         # Build_rpath is used as-is (it is usually absolute).
         if build_rpath != '':
             paths += ':' + build_rpath
-        if len(paths) < len(install_rpath):
-            padding = 'X' * (len(install_rpath) - len(paths))
-            if not paths:
-                paths = padding
-            else:
-                paths = paths + ':' + padding
+        if not paths:
+            paths = install_rpath
+        else:
+            paths = paths + ':' + install_rpath
         args = ['-Wl,-rpath,' + paths]
         if get_compiler_is_linuxlike(self):
             # Rpaths to use while linking must be absolute. These are not
diff --git a/mesonbuild/scripts/meson_install.py b/mesonbuild/scripts/meson_install.py
index 985b0e94..665f523b 100644
--- a/mesonbuild/scripts/meson_install.py
+++ b/mesonbuild/scripts/meson_install.py
@@ -353,7 +353,6 @@ def install_targets(d):
         if is_elf_platform() and os.path.isfile(outname):
             try:
                 e = depfixer.Elf(outname, False)
-                e.fix_rpath(install_rpath)
             except SystemExit as e:
                 if isinstance(e.code, int) and e.code == 0:
                     pass

jtojnar · 2017-11-11T09:05:41Z

As I understand it, the whole X business is to ensure the RPATH is not contaminated with the install_rpath but I do not see what harm it could do – on Nix, the install_rpaths will not exist in the file system (nix store) yet, and for other systems, even if they were there (if this was upstreamed, we can expect paths like /usr/local/lib/deja-dup) the $ORIGIN paths will precede the install_rpath.

lukateras · 2017-11-11T09:09:17Z

@jtojnar Thanks for explaining, makes sense. Your patch is way better than mine, and I've completely overlooked rpath templating (I shouldn't have: I've wondered why upstream version of fix_rpath works at all, considering it only works if len(old_rpath) < len(new_rpath)).

jtojnar · 2017-11-11T09:18:24Z

I asked about the patch in the meson issue mesonbuild/meson#314 (comment)

jtojnar · 2017-11-24T02:04:34Z

@yegortimoshenko The problem with this patch is that it will pollute the rpath when Nix-installed meson is used outside mkDerivation. We would need to strip the rpath on install inside meson.

lukateras · 2017-11-24T16:05:21Z

@jtojnar Does that mean that #31436 is the right way to go?

jtojnar · 2017-11-24T16:11:42Z

@yegortimoshenko What we actually want is a hybrid of the two variants: this patch plus patchelf --strip-rpath in PatchElf.fix_rpath. Edit: We might not need the strip prevention, since we would only be making the rpath shorter.

lukateras · 2017-11-24T16:17:22Z

@jtojnar In that case I think that just replacing Elf class with our version would be easier to maintain. Does this hybrid approach have practical advantages over PatchElf class in #31436?

jtojnar · 2017-11-24T16:36:31Z

@yegortimoshenko Well our PatchElf class from #31436 also does not strip the rpath so the effect is the same as for the patch above (modulo the patchelf screw ups preventing stripping).

We would need something like:

--- a/mesonbuild/compilers/compilers.py
+++ b/mesonbuild/compilers/compilers.py
@@ -834,12 +834,10 @@ class Compiler:
         # Build_rpath is used as-is (it is usually absolute).
         if build_rpath != '':
             paths += ':' + build_rpath
-        if len(paths) < len(install_rpath):
-            padding = 'X' * (len(install_rpath) - len(paths))
-            if not paths:
-                paths = padding
-            else:
-                paths = paths + ':' + padding
+        if not paths:
+            paths = install_rpath
+        else:
+            paths = paths + ':' + install_rpath
         args = ['-Wl,-rpath,' + paths]
         if get_compiler_is_linuxlike(self):
             # Rpaths to use while linking must be absolute. These are not
--- a/mesonbuild/scripts/meson_install.py
+++ b/mesonbuild/scripts/meson_install.py
@@ -353,7 +353,6 @@ def install_targets(d):
         if is_elf_platform() and os.path.isfile(outname):
             try:
-                e = depfixer.Elf(outname, False)
-                e.fix_rpath(install_rpath)
+                subprocess.call(['patchelf', '--strip-rpath', outname], stdout=subprocess.PIPE, stderr=subprocess.PIPE)

             except SystemExit as e:
                 if isinstance(e.code, int) and e.code == 0:
                     pass

We could add the stripping to the PatchElf class but the repeated patchelf calls would increase the probability of screw up.

jtojnar · 2017-12-31T08:10:58Z

Weird, the epiphany no longer finds libephysync.so, even though we are patching the rpath and according to patchelf --print-rpath it contains /nix/store/…-epiphany-3.26.4/lib/epiphany.

orivej · 2017-12-31T09:03:46Z

The issue is that libephysync.so is not loadable. You can see it with env LD_DEBUG=all. ldd .../libephysync.so prints libephymisc.so => not found.

jtojnar · 2018-01-01T04:36:50Z

The epiphany issue was fixed in c95491c.

jtojnar · 2018-02-15T02:49:33Z

As I understand it, the whole RPATH issue springs from two different requirements:

Development – we need to run the program from the build directory.
Packaging – we need to run the program from Nix store.

For third-party libraries this is fairly straight-forward – we add their locations to the rpath (or link them using absolute path on Darwin) and it will work in both scenarios. Once we want to link against an internal library, however, it will get ugly.

If we do not want to fiddle with environment variables, we will need to use absolute paths for linking. Unfortunately, the package is built in a different location than it is installed to, so we do not have a single path to use.

Instead, we could use origin-relative paths ($ORIGIN/../libdeja in figure A) but for most packages, the directory structure of the installed package will be different ($ORIGIN/../lib in figure B1 or even $ORIGIN/../../libdeja/lib in figure B2), making it as useless as absolute paths.

Obviously, a single path is not enough – we will need to add both paths to RPATH. To prevent contamination, we need to replace the dev paths for the packaging ones during during installation.

/build/deja-dup
├── deja-dup
│  └── deja-dup
└── libdeja
   └── libdeja.so

Figure A

/nix/store/deja-dup
├── bin
│  └── deja-dup
└── lib
   └── libdeja.so

Figure B1

/nix/store
├── deja-dup
│  └── bin
│     └── deja-dup
└── libdeja
   └── lib
      └── libdeja.so

Figure B2

This is actually, what meson does except it removes too much – the whole RPATH gets nuked instead of just the development paths to the internal libraries. On the other hand, our current patch removes too little – it does not remove anything at all – and on top of that, it prevents adding the local libraries to RPATH.

The ideal patch would remove only the development links and insert the install_rpath as vanilla meson does. Are absolute paths a good enough indication of non-development path to be kept?

P.S.: On Darwin, there is something similar with install_names which they use in place of RPATH (see #34445).

#28444 (comment)

jtojnar · 2018-02-23T16:00:28Z

@yegortimoshenko Here is another draft based on the previous comment:

--- a/pkgs/development/tools/build-managers/meson/default.nix
+++ b/pkgs/development/tools/build-managers/meson/default.nix
@@ -25,11 +25,14 @@
     # We patch Meson to add a --fallback-library-path argument with
     # library install_dir to g-ir-scanner.
     ./gir-fallback-path.patch
-  ];
 
-  postPatch = ''
-    sed -i -e 's|e.fix_rpath(install_rpath)||' mesonbuild/scripts/meson_install.py
-  '';
+    # In common distributions, RPATH is only needed for internal libraries so
+    # meson removes everything else. With Nix, the locations of libraries
+    # are not as predictable, therefore we need to keep them in the RPATH.
+    # At the moment we are keeping the paths starting with /nix/store.
+    # https://github.com/NixOS/nixpkgs/issues/31222#issuecomment-365811634
+    ./fix-rpath.patch
+  ];
 
   setupHook = ./setup-hook.sh;
 
--- /dev/null
+++ b/pkgs/development/tools/build-managers/meson/fix-rpath.patch
@@ -0,0 +1,45 @@
+--- a/mesonbuild/compilers/compilers.py
++++ b/mesonbuild/compilers/compilers.py
+@@ -828,6 +828,8 @@
+         return args
+ 
+     def build_unix_rpath_args(self, build_dir, from_dir, rpath_paths, build_rpath, install_rpath):
++        print(f'************* build_dir="{build_dir}", from_dir="{from_dir}", rpath_paths="{rpath_paths}", build_rpath="{build_rpath}", install_rpath="{install_rpath}"')
++
+         if not rpath_paths and not install_rpath and not build_rpath:
+             return []
+         # The rpaths we write must be relative, because otherwise
+@@ -846,8 +848,12 @@
+             if paths != '':
+                 paths += ':'
+             paths += build_rpath
+-        if len(paths) < len(install_rpath):
+-            padding = 'X' * (len(install_rpath) - len(paths))
++        print(f'------------- paths="{paths}"')
++
++        store_paths = ':'.join(filter(lambda path: path.startswith('/nix/store'), paths.split(':')))
++        extra_space_needed = len(install_rpath + (':' if install_rpath and store_paths else '') + store_paths) - len(paths)
++        if extra_space_needed > 0:
++            padding = 'X' * extra_space_needed
+             if not paths:
+                 paths = padding
+             else:
+--- a/mesonbuild/scripts/depfixer.py
++++ b/mesonbuild/scripts/depfixer.py
+@@ -300,6 +300,16 @@
+             return
+         self.bf.seek(rp_off)
+         old_rpath = self.read_str()
++
++        print(f'+++++++++++++ old_rpath="{old_rpath}", new_rpath="{new_rpath}"')
++
++        if new_rpath:
++            new_rpath += b':'
++        else:
++            new_rpath = b''
++
++        new_rpath += b':'.join(filter(lambda path: path.startswith(b'/nix/store'), old_rpath.split(b':')))
++
+         if len(old_rpath) < len(new_rpath):
+             sys.exit("New rpath must not be longer than the old one.")
+         # The linker does read-only string deduplication. If there is a

But apparently Meson is not aware of libraries added to rpath:

************* build_dir="/build/deja-dup-36.3/build", from_dir="libdeja", rpath_paths="[]", build_rpath="", install_rpath=""
************* build_dir="/build/deja-dup-36.3/build", from_dir="libdeja/tests", rpath_paths="['libdeja']", build_rpath="", install_rpath=""
------------- paths="$ORIGIN/.."
************* build_dir="/build/deja-dup-36.3/build", from_dir="libdeja/tests/unit", rpath_paths="['libdeja']", build_rpath="", install_rpath=""
------------- paths="$ORIGIN/../.."
************* build_dir="/build/deja-dup-36.3/build", from_dir="libdeja/tests/unit", rpath_paths="['libdeja']", build_rpath="", install_rpath=""
------------- paths="$ORIGIN/../.."
************* build_dir="/build/deja-dup-36.3/build", from_dir="libdeja/tools/duplicity", rpath_paths="['libdeja']", build_rpath="", install_rpath=""
------------- paths="$ORIGIN/../.."
************* build_dir="/build/deja-dup-36.3/build", from_dir="deja-dup/widgets", rpath_paths="['libdeja']", build_rpath="", install_rpath=""
------------- paths="$ORIGIN/../../libdeja"
************* build_dir="/build/deja-dup-36.3/build", from_dir="deja-dup", rpath_paths="['libdeja', 'deja-dup/widgets']", build_rpath="", install_rpath="/nix/store/gbr1sfrayang7zdggq6a5fqr2jfa8qq9-deja-dup-36.3/lib/deja-dup"
------------- paths="$ORIGIN/../libdeja:$ORIGIN/widgets"
************* build_dir="/build/deja-dup-36.3/build", from_dir="deja-dup/monitor", rpath_paths="['libdeja']", build_rpath="", install_rpath="/nix/store/gbr1sfrayang7zdggq6a5fqr2jfa8qq9-deja-dup-36.3/lib/deja-dup"
------------- paths="$ORIGIN/../../libdeja"
************* build_dir="/build/deja-dup-36.3/build", from_dir="deja-dup/nautilus", rpath_paths="[]", build_rpath="", install_rpath=""

Where do they come from? Edit: Apparently from pkgs/build-support/bintools-wrapper/ld-wrapper.sh.

In common distributions, RPATH is only needed for internal libraries so meson removes everything else. With Nix, the locations of libraries are not as predictable, therefore we need to keep them in the RPATH. [1] Previously we have just kept the RPATH produced by the linker, patching meson not to remove it. This deprived us of potentially replacing it with install_rpath provided by project so we had to re-add it manually, and also introduced a vulnerability of keeping build paths in RPATH. This commit restores the clean-up but modifies it so the items starting with /nix/store are retained. This should be relatively safe since the store is immutable, however, there might be some unwanted retainment of build_rpath [2] if it contains paths from Nix store. [1]: #31222 (comment) [2]: http://mesonbuild.com/Release-notes-for-0-42-0.html#added-build_rpath-keyword-argument

In common distributions, RPATH is only needed for internal libraries so meson removes everything else. With Nix, the locations of libraries are not as predictable, therefore we need to keep them in the RPATH. [1] Previously we have just kept the RPATH produced by the linker, patching meson not to remove it. This deprived us of potentially replacing it with install_rpath provided by project so we had to re-add it manually, and also introduced a vulnerability of keeping build paths in RPATH. This commit restores the clean-up but modifies it so the items starting with /nix/store are retained. This should be relatively safe since the store is immutable, however, there might be some unwanted retainment of build_rpath [2] if it contains paths from Nix store. [1]: NixOS#31222 (comment) [2]: http://mesonbuild.com/Release-notes-for-0-42-0.html#added-build_rpath-keyword-argument

In common distributions, RPATH is only needed for internal libraries so meson removes everything else. With Nix, the locations of libraries are not as predictable, therefore we need to keep them in the RPATH. [1] Previously we have just kept the RPATH produced by the linker, patching meson not to remove it. This deprived us of potentially replacing it with install_rpath provided by project so we had to re-add it manually, and also introduced a vulnerability of keeping build paths in RPATH. This commit restores the clean-up but modifies it so the items starting with /nix/store are retained. This should be relatively safe since the store is immutable, however, there might be some unwanted retainment of build_rpath [2] if it contains paths from Nix store. [1]: NixOS#31222 (comment) [2]: http://mesonbuild.com/Release-notes-for-0-42-0.html#added-build_rpath-keyword-argument (cherry picked from commit de910a0)

jtojnar mentioned this issue Nov 4, 2017

deja-dup: remove unneeded rpath wrapping #31219

Merged

8 tasks

jtojnar mentioned this issue Nov 4, 2017

[wip] meson: append new items to the original rpath #31228

Closed

8 tasks

lukateras mentioned this issue Nov 9, 2017

meson: append install_rpath via patchelf #31436

Closed

8 tasks

matthewbauer mentioned this issue Feb 9, 2018

macOS/darwin 'dyld: Library not loaded' for dependency - meson issue with pkgconfig and rpath #34545

Closed

jtojnar added the 0.kind: bug label Feb 15, 2018

jtojnar referenced this issue Feb 15, 2018

meson: do not strip RPATH

8f95aef

#28444 (comment)

jtojnar added this to the 18.03 milestone Feb 15, 2018

jtojnar mentioned this issue Feb 25, 2018

meson: fix rpath fixup #35513

Merged

13 tasks

FRidh closed this as completed in #35513 Mar 22, 2018

jtojnar added this to Done in Meson breakages May 16, 2018

jtojnar mentioned this issue Jun 18, 2018

Investigate new meson shared libraries patch #42155

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Meson produces incorrect rpath for local libraries #31222

Meson produces incorrect rpath for local libraries #31222

jtojnar commented Nov 4, 2017 •

edited

Loading

jtojnar commented Nov 11, 2017 •

edited

Loading

jtojnar commented Nov 11, 2017

jtojnar commented Nov 11, 2017

lukateras commented Nov 11, 2017 •

edited

Loading

jtojnar commented Nov 11, 2017

jtojnar commented Nov 24, 2017

lukateras commented Nov 24, 2017

jtojnar commented Nov 24, 2017 •

edited

Loading

lukateras commented Nov 24, 2017 •

edited

Loading

jtojnar commented Nov 24, 2017 •

edited

Loading

jtojnar commented Dec 31, 2017 •

edited

Loading

orivej commented Dec 31, 2017

jtojnar commented Jan 1, 2018

jtojnar commented Feb 15, 2018 •

edited

Loading

jtojnar commented Feb 23, 2018 •

edited

Loading

Meson produces incorrect rpath for local libraries #31222

Meson produces incorrect rpath for local libraries #31222

Comments

jtojnar commented Nov 4, 2017 • edited Loading

Issue description

Steps to reproduce

Technical details

jtojnar commented Nov 11, 2017 • edited Loading

jtojnar commented Nov 11, 2017

jtojnar commented Nov 11, 2017

lukateras commented Nov 11, 2017 • edited Loading

jtojnar commented Nov 11, 2017

jtojnar commented Nov 24, 2017

lukateras commented Nov 24, 2017

jtojnar commented Nov 24, 2017 • edited Loading

lukateras commented Nov 24, 2017 • edited Loading

jtojnar commented Nov 24, 2017 • edited Loading

jtojnar commented Dec 31, 2017 • edited Loading

orivej commented Dec 31, 2017

jtojnar commented Jan 1, 2018

jtojnar commented Feb 15, 2018 • edited Loading

jtojnar commented Feb 23, 2018 • edited Loading

jtojnar commented Nov 4, 2017 •

edited

Loading

jtojnar commented Nov 11, 2017 •

edited

Loading

lukateras commented Nov 11, 2017 •

edited

Loading

jtojnar commented Nov 24, 2017 •

edited

Loading

lukateras commented Nov 24, 2017 •

edited

Loading

jtojnar commented Nov 24, 2017 •

edited

Loading

jtojnar commented Dec 31, 2017 •

edited

Loading

jtojnar commented Feb 15, 2018 •

edited

Loading

jtojnar commented Feb 23, 2018 •

edited

Loading