From: Christopher Baines <mail@cbaines.net>
To: Marius Bakke <mbakke@fastmail.com>
Cc: 36535@debbugs.gnu.org
Subject: [bug#36535] [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch.
Date: Mon, 08 Jul 2019 16:59:13 +0100 [thread overview]
Message-ID: <87bly4a566.fsf@cbaines.net> (raw)
In-Reply-To: <875zocr4hl.fsf@devup.no>
[-- Attachment #1: Type: text/plain, Size: 9043 bytes --]
Marius Bakke <mbakke@fastmail.com> writes:
> Hi Chris,
>
> Christopher Baines <mail@cbaines.net> writes:
>
>> Christopher Baines <mail@cbaines.net> writes:
>>
>>> Incorporate some changes from nixpkgs to the gobject-introspection package
>>> patches. This is motivated by looking at issues with libsoup and lollypop.
>>> This changes means that the share/gir-1.0/Soup-2.4.gir file within libsoup
>>> references libsoup-2.4.so.1 with an absolute filename, whereas previously, the
>>> filename wasn't absolute.
>>>
>>> * gnu/packages/patches/gobject-introspection-absolute-shlib-path.patch:
>>> Incorporate changes from nixpkgs.
>>> ---
>>> ...ct-introspection-absolute-shlib-path.patch | 141 +++++++++++++++++-
>>> 1 file changed, 137 insertions(+), 4 deletions(-)
>>>
>>
>> I've pushed this as [1] to core-updates now, as I wanted to get it in
>> before the freeze.
>
> Thank you for addressing this. IIUC previously lollypop failed to
> retain a reference to libsoup-2.4.so.1, whereas with this patch it does?
Not quite... I think lollypop was reading the typelib in libsoup, but
the shared library was just referenced by filename, not the absolute
filename, and I think this was causing issues when trying to use libsoup
from lollypop.
On master:
grep shared-library /gnu/store/bafaiiblr2vmmf1zvidkw1137ndqnqg2-libsoup-2.66.2/share/gir-1.0/Soup-2.4.gir
shared-library="libsoup-2.4.so.1"
On core-updates:
grep shared-library /gnu/store/b1ykh6xj11v7zav4r68v8qflk31cnddm-libsoup-2.66.2/share/gir-1.0/Soup-2.4.gir
shared-library="/gnu/store/b1ykh6xj11v7zav4r68v8qflk31cnddm-libsoup-2.66.2/lib/libsoup-2.4.so.1"
> A few comments about the patch:
>
>> diff --git a/gnu/packages/patches/gobject-introspection-absolute-shlib-path.patch b/gnu/packages/patches/gobject-introspection-absolute-shlib-path.patch
>> index d00cc5a420..3c0bb1c6cf 100644
>> --- a/gnu/packages/patches/gobject-introspection-absolute-shlib-path.patch
>> +++ b/gnu/packages/patches/gobject-introspection-absolute-shlib-path.patch
>> @@ -2,10 +2,131 @@
>> # add the full path.
>> #
>> # This patch was provided by Luca Bruno <lucabru@src.gnome.org> for
>> -# 'gobject-introspection' 1.40.0 in Nix.
>> ---- ./giscanner/utils.py.orig 2014-08-14 22:05:05.055334080 +0200
>> -+++ ./giscanner/utils.py 2014-08-14 22:05:24.687497334 +0200
>> -@@ -110,17 +110,11 @@
>> +# 'gobject-introspection' 1.40.0 in Nix.
>> +#
>> +# It has since been updated to work with newer versions of
>> +# gobject-introspection.
>> +--- a/giscanner/scannermain.py
>> ++++ b/giscanner/scannermain.py
>> +@@ -95,6 +95,39 @@ def get_windows_option_group(parser):
>> + return group
>> +
>> +
>> ++def _get_default_fallback_libpath():
>> ++ # Newer multiple-output-optimized stdenv has an environment variable
>> ++ # $outputLib which in turn specifies another variable which then is used as
>> ++ # the destination for the library contents (${!outputLib}/lib).
>> ++ store_path = os.environ.get(os.environ.get("outputLib")) if "outputLib" in os.environ else None
>> ++ if store_path is None:
>> ++ outputs = os.environ.get("outputs", "out").split()
>
> gnu-build-system does not currently export an "outputs" variable.
> Perhaps it should?
Ah, I didn't realise this part of the patch was as Nix specific as it
is...
At least for the change I was trying to affect, this seems to be
probably redundant, or somehow doing the job. Maybe this part of the
patch relating to the fallback_libpath should be removed.
>> ++ if "lib" in outputs:
>> ++ # For multiple output derivations let's try whether there is a $lib
>> ++ # environment variable and use that as the base store path.
>> ++ store_path = os.environ.get("lib")
>> ++ elif "out" in outputs:
>> ++ # Otherwise we have a single output derivation, so the libraries
>> ++ # most certainly will end up in "$out/lib".
>> ++ store_path = os.environ.get("out")
>
> Consequently, this is the only ever matching case, and "lib" outputs are
> ignored, counter to what one might expect from glancing over this patch.
>
> That is, unless one sets an "outputs" or "outputLib" variable in a
> package recipe, so maybe we don't have to do anything here.
>
>> ++
>> ++ if store_path is not None:
>> ++ # Even if we have a $lib as output, there still should be a $lib/lib
>> ++ # directory.
>> ++ return os.path.join(store_path, 'lib')
>> ++ else:
>> ++ # If we haven't found a possible scenario, let's return an empty string
>> ++ # so that the shared library won't be prepended with a path.
>> ++ #
>> ++ # Note that this doesn't mean that all hope is lost, because after all
>> ++ # we can still use --fallback-library-path to set one.
>> ++ #
>> ++ # Also, we're not returning None, because that would make it very
>> ++ # difficult to disable adding fallback paths altogether using something
>> ++ # like: --fallback-library-path=""
>> ++ return ""
>> ++
>> ++
>> + def _get_option_parser():
>> + parser = optparse.OptionParser('%prog [options] sources',
>> + version='%prog ' + giscanner.__version__)
>> +@@ -205,6 +238,10 @@ match the namespace prefix.""")
>> + parser.add_option("", "--filelist",
>> + action="store", dest="filelist", default=[],
>> + help="file containing headers and sources to be scanned")
>> ++ parser.add_option("", "--fallback-library-path",
>> ++ action="store", dest="fallback_libpath",
>> ++ default=_get_default_fallback_libpath(),
>> ++ help="Path to prepend to unknown shared libraries")
>> +
>> + group = get_preprocessor_option_group(parser)
>> + parser.add_option_group(group)
>> +--- a/giscanner/shlibs.py
>> ++++ b/giscanner/shlibs.py
>> +@@ -57,6 +57,12 @@ def _ldd_library_pattern(library_name):
>> + $""" % re.escape(library_name), re.VERBOSE)
>> +
>> +
>> ++def _ldd_library_guix_pattern(library_name):
>> ++ store_dir = re.escape('/gnu/store')
>
> Here we should use:
>
> os.environ.get("NIX_STORE") if "NIX_STORE" in os.environ else "/gnu/store"
>
> So that it works for non-default store prefixes.
Given NIX_STORE is set at build time, and this code is mostly used at
build time, then that would work.
Before I was thinking about how to actually put the store path in the
code at build time, but that's probably not necessary.
>> ++ pattern = r'(%s(?:/[^/]*)+lib%s[^A-Za-z0-9_-][^\s\(\)]*)'
>> ++ return re.compile(pattern % (store_dir, re.escape(library_name)))
>> ++
>> ++
>> + # This is a what we do for non-la files. We assume that we are on an
>> + # ELF-like system where ldd exists and the soname extracted with ldd is
>> + # a filename that can be opened with dlopen().
>> +@@ -106,7 +112,8 @@ def _resolve_non_libtool(options, binary, libraries):
>> + output = output.decode("utf-8", "replace")
>> +
>> + shlibs = resolve_from_ldd_output(libraries, output)
>> +- return list(map(sanitize_shlib_path, shlibs))
>> ++ fallback_libpath = options.fallback_libpath or "";
>> ++ return list(map(lambda p: os.path.join(fallback_libpath, p), map(sanitize_shlib_path, shlibs)))
>> +
>> +
>> + def sanitize_shlib_path(lib):
>> +@@ -115,19 +122,18 @@ def sanitize_shlib_path(lib):
>> + # In case we get relative paths on macOS (like @rpath) then we fall
>> + # back to the basename as well:
>> + # https://gitlab.gnome.org/GNOME/gobject-introspection/issues/222
>> +- if sys.platform == "darwin":
>> +- if not os.path.isabs(lib):
>> +- return os.path.basename(lib)
>> +- return lib
>> +- else:
>> ++
>> ++ # Always use absolute paths if available
>> ++ if not os.path.isabs(lib):
>> + return os.path.basename(lib)
>> ++ return lib
>> +
>> +
>> + def resolve_from_ldd_output(libraries, output):
>> + patterns = {}
>> + for library in libraries:
>> + if not os.path.isfile(library):
>> +- patterns[library] = _ldd_library_pattern(library)
>> ++ patterns[library] = (_ldd_library_pattern(library), _ldd_library_guix_pattern(library))
>> + if len(patterns) == 0:
>> + return []
>> +
>> +@@ -139,8 +145,11 @@ def resolve_from_ldd_output(libraries, output):
>> + if line.endswith(':'):
>> + continue
>> + for word in line.split():
>> +- for library, pattern in patterns.items():
>> +- m = pattern.match(word)
>> ++ for library, (pattern, guix_pattern) in patterns.items():
>> ++ if line.find('/gnu/store') != -1:
>
> Use $NIX_STORE here, too.
>
> Other than that LGTM.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]
next prev parent reply other threads:[~2019-07-08 16:00 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-07 10:48 [bug#36535] [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch Christopher Baines
2019-07-08 7:46 ` bug#36535: " Christopher Baines
2019-07-08 14:21 ` [bug#36535] " Marius Bakke
2019-07-08 15:59 ` Christopher Baines [this message]
2019-07-08 16:29 ` Marius Bakke
2019-07-10 17:35 ` Marius Bakke
2019-07-12 18:44 ` Marius Bakke
2019-07-12 23:22 ` Christopher Baines
2019-07-13 11:09 ` [bug#36535] [PATCH] gnu: gobject-introspection: Remove hardcoded store from patch Christopher Baines
2019-07-13 11:11 ` [bug#36535] [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch Christopher Baines
2019-07-13 16:46 ` Marius Bakke
2019-07-13 22:14 ` bug#36535: " Christopher Baines
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87bly4a566.fsf@cbaines.net \
--to=mail@cbaines.net \
--cc=36535@debbugs.gnu.org \
--cc=mbakke@fastmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).