gobject-introspection: Use absolute shared library paths #12558

aszlig · 2016-01-22T21:57:41Z

So far only .la files get correctly converted to absolute paths in the GIR file. However if there are .so files which depend on a particular library using GI, they still get only the basename of the .so file.

This improves on the existing absolute_shlib_path.patch not only figuring out the absolute path of .so files but also falling back on the absolute path of $out/lib (or $lib/lib with multiple outputs) of the current build.

With this, we should no longer need to resort to setting LD_LIBRARY_PATH for all programs that use GI libraries.

I have only done very limited testing and I'm not using Gnome, nor do I use OS X, so I don't know whether this will cause problems on any of these environments.

Cc: @lethalman and anyone who can test on Darwin :-)

The gi-r-scanner is generating a list of shared libraries that are referenced in the shared-library attribute of the <namespace/> element of the GIR file. However, this attribute only contains the names of the libraries and not the full store paths, like for example while preparing to package libblockdev, the following items were included in the shared-library attribute: /nix/store/...-libblockdev-1.3/lib/libblockdev.so.0 libm.so.6 libdmraid.so.1.0.0.rc16 libbd_utils.so.0 Unfortunately, loading such a library without setting LD_LIBRARY_PATH is going to fail finding libm.so.6 and libdmraid.so.1.0.0.rc16. Now the first attempt at solving this was to put absolute paths of all the libraries referenced in the shared-library attribute, but this also led up to including paths of build-time shared objects into that attribute: /nix/store/...-libblockdev-1.3/lib/libblockdev.so.0 /nix/store/...-glibc-2.21/lib/libm.so.6 /nix/store/...-dmraid-1.0.0.rc16/lib/libdmraid.so.1.0.0.rc16 /tmp/nix-build-libblockdev-1.3.drv-0/.../utils/.libs/libbd_utils.so.0 This of course is not what we want, so the final solution is to only use the absolute path whenever it is a Nix path and leave the library name as-is if the path doesn't reside within the store, like this: /nix/store/...-libblockdev-1.3/lib/libblockdev.so.0 /nix/store/...-glibc-2.21/lib/libm.so.6 /nix/store/...-dmraid-1.0.0.rc16/lib/libdmraid.so.1.0.0.rc16 libbd_utils.so.0 The downside of this approach is that if not even the output path of the library is in LD_LIBRARY_PATH, even loading of libbd_utils.so.0 could fail, so we need to patch the loader as well. Signed-off-by: aszlig <aszlig@redmoonstudios.org>

After patching up the shared libraries in c420de6 to use absolute paths, there are still some libraries left which do not get an absolute paths assigned. Those libraries are the ones which have an absolute path outside of the Nix store, so we assume that they're build products of the current build and make them absolute by prepending "$out/lib" or "$lib/lib" (depending on whether it's a multiple output derivation or not) to its basename. So for my test case, the resulting library paths now look like this: /nix/store/...-libblockdev-1.3/lib/libblockdev.so.0 /nix/store/...-glibc-2.21/lib/libm.so.6 /nix/store/...-dmraid-1.0.0.rc16/lib/libdmraid.so.1.0.0.rc16 /nix/store/...-libblockdev-1.3/lib/libbd_utils.so.0 Which is perfectly fine and everything gets resolved correctly after importing the library using GI. However, I didn't test it against other libraries and programs, so this still needs testing, especially for Darwin. Signed-off-by: aszlig <aszlig@redmoonstudios.org>

mention-bot · 2016-01-22T21:57:51Z

By analyzing the blame information on this pull request, we identified @lethalman, @lovek323 and @vcunat to be potential reviewers

lucabrunox · 2016-01-22T22:07:47Z

Which package is having problems such that we need this patch?

aszlig · 2016-01-22T22:12:07Z

@lethalman: A package that is not yet in <nixpkgs>, but will end up there soon (libblockdev). This is less about a specific package but more of a generic fix to avoid ugly hacks with LD_LIBRARY_PATH or even postprocessing using sed.

lucabrunox · 2016-01-22T22:14:50Z

@aszlig I'd like to understand the use case. Most of GI packages already work fine because the GIR has an absolute file with that patch. Would you elaborate on your case and why it doesn't work exactly?

aszlig · 2016-01-22T22:19:42Z

@lethalman: The GIR file has an absolute path, but not the shared libraries referenced in <namespace shared-library="...">, thus all of the dependencies of the typelib have to be in LD_LIBRARY_PATH or added with other hacks like for example this one.

lucabrunox · 2016-01-22T22:23:00Z

pkgs/development/libraries/gobject-introspection/absolute_shlib_path.patch

+          (?:lib%s[^A-Za-z0-9_-][^\s\(\)]*)
+        )
+    '''
+    return re.compile(pattern % (nix_store_dir, re.escape(library_name)),


Why not just leave absolute paths as absolute without checking for the exact nix store path? Didn't understand the comment.

Because absolute paths weren't used in the first place. It's parsing the output of ldd and just added all the basenames of the libraries found there.

The reason for still retaining the upstream behavior if it's not a Nix path is that the absolute paths of the temporary build directory will end up being there as well, see the commit message of c420de6.

I've improved the comment to make it hopefully more clear.

@lethalman

If no config.nix.storeDir has been set, don't fall back to "/nix/store" but use builtins.storeDir instead so we always should end up with the correct store path no matter whether config.nix.storeDir has been set. Thanks to @lethalman for pointing this out. Signed-off-by: aszlig <aszlig@redmoonstudios.org>

@lethalman

As the comment needed explanation, that it's about temporary build files, this should do better. Thanks again to @lethalman for pointing that out. Signed-off-by: aszlig <aszlig@redmoonstudios.org>

@lethalman

Once NixOS#7701 gets merged, we have another environment variable called $outputLib, which then points to another environment variable which is the final library output. This was brought up in discussion with @lethalman and @vcunat in: NixOS#12558 (comment) The closure-size branch is not yet merged into master, so this is only a preparation and we're still falling back to $out and $lib whenever $outputLib isn't available. Signed-off-by: aszlig <aszlig@redmoonstudios.org>

lucabrunox · 2016-01-23T15:53:23Z

Tomorrow I'll try running gnome with this change.

aszlig · 2016-02-03T14:58:19Z

@lethalman: Did it work out well?

lucabrunox · 2016-02-03T19:20:10Z

@aszlig I cannot boot the latest unstable on virtualbox so I cannot test at the moment... if you feel it's working, or if anybody can test it on gnome3 and see that most apps work fine, feel free to push.

aszlig · 2016-02-07T14:46:40Z

@lethalman: Tested this against the following VM config and it seems to work fine, though I'm not a Gnome power user (or better: no Gnome user at all), so my tests probably aren't comprehensive enough:

{
  services.xserver.enable = true;
  services.xserver.displayManager.gdm.enable = true;
  services.xserver.desktopManager.gnome3.enable = true;

  users.users.test = {
    password = "test";
    isNormalUser = true;
  };
}

Anything in particular I should test?

lucabrunox · 2016-02-07T14:50:01Z

@aszlig as long as most gnome apps are able to open (like gnome-tweak-tool, music, photos, clocks), it's ok.

So far only .la files get correctly converted to absolute paths in the GIR file. However if there are .so files which depend on a particular library using GI, they still get only the basename of the .so file. This improves on the existing absolute_shlib_path.patch not only figuring out the absolute path of .so files but also falling back on the absolute path of $out/lib (or $lib/lib with multiple outputs) of the current build. With this, we should no longer need to resort to setting LD_LIBRARY_PATH for all programs that use GI libraries. I'm merging this because after more than a month no issues came up so far.

aszlig added 2 commits January 22, 2016 22:46

aszlig added 0.kind: enhancement 9.needs: reporter feedback 6.topic: darwin Running or building packages on Darwin 6.topic: GNOME GNOME desktop environment and its underlying platform labels Jan 22, 2016

lucabrunox reviewed Jan 22, 2016
View reviewed changes

aszlig added 3 commits January 22, 2016 23:55

gobject-introspection: Improve comment in patch

b3b444a

As the comment needed explanation, that it's about temporary build files, this should do better. Thanks again to @lethalman for pointing that out. Signed-off-by: aszlig <aszlig@redmoonstudios.org>

aszlig merged commit 740b30b into NixOS:master Mar 1, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gobject-introspection: Use absolute shared library paths #12558

gobject-introspection: Use absolute shared library paths #12558

aszlig commented Jan 22, 2016

mention-bot commented Jan 22, 2016

lucabrunox commented Jan 22, 2016

aszlig commented Jan 22, 2016

lucabrunox commented Jan 22, 2016

aszlig commented Jan 22, 2016

lucabrunox Jan 22, 2016

aszlig Jan 22, 2016

aszlig Jan 22, 2016

lucabrunox commented Jan 23, 2016

aszlig commented Feb 3, 2016

lucabrunox commented Feb 3, 2016

aszlig commented Feb 7, 2016

lucabrunox commented Feb 7, 2016

gobject-introspection: Use absolute shared library paths #12558

gobject-introspection: Use absolute shared library paths #12558

Conversation

aszlig commented Jan 22, 2016

mention-bot commented Jan 22, 2016

lucabrunox commented Jan 22, 2016

aszlig commented Jan 22, 2016

lucabrunox commented Jan 22, 2016

aszlig commented Jan 22, 2016

lucabrunox Jan 22, 2016

Choose a reason for hiding this comment

aszlig Jan 22, 2016

Choose a reason for hiding this comment

aszlig Jan 22, 2016

Choose a reason for hiding this comment

lucabrunox commented Jan 23, 2016

aszlig commented Feb 3, 2016

lucabrunox commented Feb 3, 2016

aszlig commented Feb 7, 2016

lucabrunox commented Feb 7, 2016