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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 36535 in the body.
You can then email your comments to 36535 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Christopher Baines <mail <at> cbaines.net>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: gobject-introspection: Update absolute-shlib-path.patch.
Date: Sun,  7 Jul 2019 11:48:03 +0100

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(-)

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 <at> 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()
++        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")
++
++    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')
++    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:
++                    m = guix_pattern.match(word)
++                else:
++                    m = pattern.match(word)
+                 if m:
+                     del patterns[library]
+                     shlibs.append(m.group())
+
+--- a/giscanner/utils.py
++++ b/giscanner/utils.py
+@@ -111,17 +111,11 @@ def extract_libtool_shlib(la_file):
      if dlname is None:
          return None
  
@@ -28,3 +149,15 @@
  
  
  def extract_libtool(la_file):
+--- a/tests/scanner/test_shlibs.py
++++ b/tests/scanner/test_shlibs.py
+@@ -40,6 +64,7 @@ class TestLddParser(unittest.TestCase):
+ 
+         self.assertEqual(
+             sanitize_shlib_path('/foo/bar'),
+-            '/foo/bar' if sys.platform == 'darwin' else 'bar')
++            # Always use an absolute filename for Guix
++            '/foo/bar')
+ 
+     def test_unresolved_library(self):
+output = ''
-- 
2.22.0

Message #10 received at 36535-done <at> debbugs.gnu.org (full text, mbox):

From: Christopher Baines <mail <at> cbaines.net>
To: 36535-done <at> debbugs.gnu.org
Subject: Re: [bug#36535] [PATCH] gnu: gobject-introspection: Update
 absolute-shlib-path.patch.
Date: Mon, 08 Jul 2019 08:46:46 +0100

[Message part 1 (text/plain, inline)]

Christopher Baines <mail <at> 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.

1: 8747477deb765571c300d3eb9a4012a3c36cf7f7

[signature.asc (application/pgp-signature, inline)]

Message #13 received at 36535-done <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Christopher Baines <mail <at> cbaines.net>, 36535-done <at> debbugs.gnu.org
Subject: Re: bug#36535: [PATCH] gnu: gobject-introspection: Update
 absolute-shlib-path.patch.
Date: Mon, 08 Jul 2019 16:21:58 +0200

[Message part 1 (text/plain, inline)]

Hi Chris,

Christopher Baines <mail <at> cbaines.net> writes:

> Christopher Baines <mail <at> 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?

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 <at> 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?

> ++        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.

> ++    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 (application/pgp-signature, inline)]

Message #16 received at 36535 <at> debbugs.gnu.org (full text, mbox):

From: Christopher Baines <mail <at> cbaines.net>
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 36535 <at> debbugs.gnu.org
Subject: Re: bug#36535: [PATCH] gnu: gobject-introspection: Update
 absolute-shlib-path.patch.
Date: Mon, 08 Jul 2019 16:59:13 +0100

[Message part 1 (text/plain, inline)]

Marius Bakke <mbakke <at> fastmail.com> writes:

> Hi Chris,
>
> Christopher Baines <mail <at> cbaines.net> writes:
>
>> Christopher Baines <mail <at> 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 <at> 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.

[signature.asc (application/pgp-signature, inline)]

Message #19 received at 36535 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Christopher Baines <mail <at> cbaines.net>
Cc: 36535 <at> debbugs.gnu.org
Subject: Re: bug#36535: [PATCH] gnu: gobject-introspection: Update
 absolute-shlib-path.patch.
Date: Mon, 08 Jul 2019 18:29:56 +0200

[Message part 1 (text/plain, inline)]

Christopher Baines <mail <at> cbaines.net> writes:

> Marius Bakke <mbakke <at> fastmail.com> writes:
>
>> Hi Chris,
>>
>> Christopher Baines <mail <at> cbaines.net> writes:
>>
>>> Christopher Baines <mail <at> 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.

I see, thanks for explaining.  In Guix, we usually resolve these
situations by native-search-paths, do you know if gobject-introspection
supports looking up the 'share/gir-1.0' directory from an environment
variable (similar to how GI_TYPELIB_PATH works today)?  However...

>
> 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"

...this is even better, so I am mostly just curious :-)

>> 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 <at> 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.

I'd keep the "$outputs" logic, it sounds like a useful and easy change
to do in gnu-build-system, although maybe not for this 'core-updates'
round.  We can use it in package recipes for fun and profit meanwhile.

However I doubt we'll ever use "outputLib", so it would be good to
remove that.

If you are updating the patch, could you also add a link to the upstream
patch, as well as one to this discussion?

Thank you!

[signature.asc (application/pgp-signature, inline)]

Message #22 received at 36535 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Christopher Baines <mail <at> cbaines.net>
Cc: 36535 <at> debbugs.gnu.org
Subject: Re: bug#36535: [PATCH] gnu: gobject-introspection: Update
 absolute-shlib-path.patch.
Date: Wed, 10 Jul 2019 19:35:21 +0200

[Message part 1 (text/plain, inline)]

Marius Bakke <mbakke <at> fastmail.com> writes:

> Christopher Baines <mail <at> cbaines.net> writes:
>
>> Marius Bakke <mbakke <at> fastmail.com> writes:
>>
>>> Hi Chris,
>>>
>>> Christopher Baines <mail <at> cbaines.net> writes:
>>>
>>>> Christopher Baines <mail <at> 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.
>
> I see, thanks for explaining.  In Guix, we usually resolve these
> situations by native-search-paths, do you know if gobject-introspection
> supports looking up the 'share/gir-1.0' directory from an environment
> variable (similar to how GI_TYPELIB_PATH works today)?  However...

Errh, ignore this, I need to study the GObject stuff one of these days...

Do you think you'll have time to update the patch within the coming
days?  The only important part is the NIX_STORE bits; the rest can be
dealt with later.

TIA!

[signature.asc (application/pgp-signature, inline)]

Message #27 received at 36535 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Christopher Baines <mail <at> cbaines.net>
Cc: 36535 <at> debbugs.gnu.org
Subject: Re: bug#36535: [PATCH] gnu: gobject-introspection: Update
 absolute-shlib-path.patch.
Date: Fri, 12 Jul 2019 20:44:08 +0200

severity 36535 important

>>>> ++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.
>
> I'd keep the "$outputs" logic, it sounds like a useful and easy change
> to do in gnu-build-system, although maybe not for this 'core-updates'
> round.  We can use it in package recipes for fun and profit meanwhile.

We now have a user of the $outputs variable:

https://git.savannah.gnu.org/cgit/guix.git/commit/?id=7555d539245ff3456848c02d61f9e601ee5af463

Incidentally, the package was broken because of the very same feature :-)

But we can not merge this with the hard-coded /gnu/store paths, as that
is likely to cause strange problems for users with a non-default store
prefix.

Unless someone steps up to fix it within a few days, I think we'll have
to revert it for now.

Message #30 received at 36535 <at> debbugs.gnu.org (full text, mbox):

From: Christopher Baines <mail <at> cbaines.net>
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 36535 <at> debbugs.gnu.org
Subject: Re: bug#36535: [PATCH] gnu: gobject-introspection: Update
 absolute-shlib-path.patch.
Date: Sat, 13 Jul 2019 00:22:59 +0100

[Message part 1 (text/plain, inline)]

Marius Bakke <mbakke <at> fastmail.com> writes:

> severity 36535 important
>
>>>>> ++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.
>>
>> I'd keep the "$outputs" logic, it sounds like a useful and easy change
>> to do in gnu-build-system, although maybe not for this 'core-updates'
>> round.  We can use it in package recipes for fun and profit meanwhile.
>
> We now have a user of the $outputs variable:
>
> https://git.savannah.gnu.org/cgit/guix.git/commit/?id=7555d539245ff3456848c02d61f9e601ee5af463

Ooh, interesting :)

> Incidentally, then package was broken because of the very same feature :-)
>
> But we can not merge this with the hard-coded /gnu/store paths, as that
> is likely to cause strange problems for users with a non-default store
> prefix.
>
> Unless someone steps up to fix it within a few days, I think we'll have
> to revert it for now.

I have some time now to look at this, I'm currently building libsoup
with these changes to the patch.


modified   gnu/packages/patches/gobject-introspection-absolute-shlib-path.patch
@@ -61,12 +61,14 @@
      parser.add_option_group(group)
 --- a/giscanner/shlibs.py
 +++ b/giscanner/shlibs.py
-@@ -57,6 +57,12 @@ def _ldd_library_pattern(library_name):
+@@ -57,6 +57,14 @@ 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')
++    store_dir = re.escape(
++      os.environ.get("NIX_STORE", default="/gnu/store")
++    )
 +    pattern = r'(%s(?:/[^/]*)+lib%s[^A-Za-z0-9_-][^\s\(\)]*)'
 +    return re.compile(pattern % (store_dir, re.escape(library_name)))
 +
@@ -109,14 +111,15 @@
      if len(patterns) == 0:
          return []
  
-@@ -139,8 +145,11 @@ def resolve_from_ldd_output(libraries, output):
+@@ -139,8 +145,12 @@ 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:
++                store_dir = os.environ.get("NIX_STORE", default="/gnu/store")
++                if line.find(store_dir) != -1:
 +                    m = guix_pattern.match(word)
 +                else:
 +                    m = pattern.match(word)

[signature.asc (application/pgp-signature, inline)]

Message #33 received at 36535 <at> debbugs.gnu.org (full text, mbox):

From: Christopher Baines <mail <at> cbaines.net>
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 36535 <at> debbugs.gnu.org
Subject: [PATCH] gnu: gobject-introspection: Remove hardcoded store from patch.
Date: Sat, 13 Jul 2019 12:09:50 +0100

* gnu/packages/patches/gobject-introspection-absolute-shlib-path.patch: Use
the NIX_STORE environment variable, rather than hardcoding the store
directory.
---
 .../gobject-introspection-absolute-shlib-path.patch   | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/gnu/packages/patches/gobject-introspection-absolute-shlib-path.patch b/gnu/packages/patches/gobject-introspection-absolute-shlib-path.patch
index 3c0bb1c6cf..956fa617c3 100644
--- a/gnu/packages/patches/gobject-introspection-absolute-shlib-path.patch
+++ b/gnu/packages/patches/gobject-introspection-absolute-shlib-path.patch
@@ -61,12 +61,14 @@
      parser.add_option_group(group)
 --- a/giscanner/shlibs.py
 +++ b/giscanner/shlibs.py
-@@ -57,6 +57,12 @@ def _ldd_library_pattern(library_name):
+@@ -57,6 +57,14 @@ 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')
++    store_dir = re.escape(
++      os.environ.get("NIX_STORE", default="/gnu/store")
++    )
 +    pattern = r'(%s(?:/[^/]*)+lib%s[^A-Za-z0-9_-][^\s\(\)]*)'
 +    return re.compile(pattern % (store_dir, re.escape(library_name)))
 +
@@ -109,14 +111,15 @@
      if len(patterns) == 0:
          return []
  
-@@ -139,8 +145,11 @@ def resolve_from_ldd_output(libraries, output):
+@@ -139,8 +145,12 @@ 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:
++                store_dir = os.environ.get("NIX_STORE", default="/gnu/store")
++                if line.find(store_dir) != -1:
 +                    m = guix_pattern.match(word)
 +                else:
 +                    m = pattern.match(word)
-- 
2.22.0

Message #36 received at 36535 <at> debbugs.gnu.org (full text, mbox):

From: Christopher Baines <mail <at> cbaines.net>
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 36535 <at> debbugs.gnu.org
Subject: Re: [bug#36535] [PATCH] gnu: gobject-introspection: Update
 absolute-shlib-path.patch.
Date: Sat, 13 Jul 2019 12:11:09 +0100

[Message part 1 (text/plain, inline)]

Christopher Baines <mail <at> cbaines.net> writes:

> I have some time now to look at this, I'm currently building libsoup
> with these changes to the patch.

I've now send a patch with these changes. I've managed to build libsoup
with it, and the .gir files look good.

[signature.asc (application/pgp-signature, inline)]

Message #39 received at 36535 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Christopher Baines <mail <at> cbaines.net>
Cc: 36535 <at> debbugs.gnu.org
Subject: Re: [bug#36535] [PATCH] gnu: gobject-introspection: Update
 absolute-shlib-path.patch.
Date: Sat, 13 Jul 2019 18:46:23 +0200

[Message part 1 (text/plain, inline)]

Christopher Baines <mail <at> cbaines.net> writes:

> Christopher Baines <mail <at> cbaines.net> writes:
>
>> I have some time now to look at this, I'm currently building libsoup
>> with these changes to the patch.
>
> I've now send a patch with these changes. I've managed to build libsoup
> with it, and the .gir files look good.

Thank you!  The changes LGTM.

[signature.asc (application/pgp-signature, inline)]

Message #44 received at 36535-done <at> debbugs.gnu.org (full text, mbox):

From: Christopher Baines <mail <at> cbaines.net>
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 36535-done <at> debbugs.gnu.org
Subject: Re: [bug#36535] [PATCH] gnu: gobject-introspection: Update
 absolute-shlib-path.patch.
Date: Sat, 13 Jul 2019 23:14:18 +0100

[Message part 1 (text/plain, inline)]

Marius Bakke <mbakke <at> fastmail.com> writes:

> Christopher Baines <mail <at> cbaines.net> writes:
>
>> Christopher Baines <mail <at> cbaines.net> writes:
>>
>>> I have some time now to look at this, I'm currently building libsoup
>>> with these changes to the patch.
>>
>> I've now send a patch with these changes. I've managed to build libsoup
>> with it, and the .gir files look good.
>
> Thank you!  The changes LGTM.

Great, I've pushed this to core-updates now.

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.