[bug#36535] [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch.

[Christopher Baines]

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