[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#36535] [PATCH] gnu: gobject-introspection: Update absolute-shlib-pa
From: |
Christopher Baines |
Subject: |
[bug#36535] [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch. |
Date: |
Mon, 08 Jul 2019 16:59:13 +0100 |
User-agent: |
mu4e 1.2.0; emacs 26.2 |
Marius Bakke <address@hidden> writes:
> Hi Chris,
>
> Christopher Baines <address@hidden> writes:
>
>> Christopher Baines <address@hidden> 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 <address@hidden> 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.
signature.asc
Description: PGP signature
- [bug#36535] [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch., Christopher Baines, 2019/07/07
- bug#36535: [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch., Christopher Baines, 2019/07/08
- [bug#36535] [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch., Marius Bakke, 2019/07/08
- [bug#36535] [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch.,
Christopher Baines <=
- [bug#36535] [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch., Marius Bakke, 2019/07/08
- [bug#36535] [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch., Marius Bakke, 2019/07/10
- [bug#36535] [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch., Marius Bakke, 2019/07/12
- [bug#36535] [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch., Christopher Baines, 2019/07/12
- [bug#36535] [PATCH] gnu: gobject-introspection: Remove hardcoded store from patch., Christopher Baines, 2019/07/13
- [bug#36535] [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch., Christopher Baines, 2019/07/13
- [bug#36535] [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch., Marius Bakke, 2019/07/13
- bug#36535: [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch., Christopher Baines, 2019/07/13