bug#31210: Perl graft breakage

bug#31210: Perl graft breakage

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 5631 bytes --]

On Sun, Apr 15, 2018 at 05:34:05PM +0200, Marius Bakke wrote:
> * gnu/packages/perl.scm (perl-5.26.2): New public variable.
> (perl)[replacement]: New field.

I noticed on my Debian system that this broke rxvt-unicode, and it
probably affects other packages as well.

The issue is that the full path of libperl.so includes the Perl version
number, for example:

/gnu/store/...-perl-5.26.1/lib/perl5/5.26.1/x86_64-linux-thread-multi/CORE/libperl.so

The RUNPATH of urxvt with the Perl graft includes this bogus path:

/gnu/store/ynk3rg3i791i6k7rz2imbyj9wyfjrnrc-perl-5.26.2/lib/perl5/5.26.1/x86_64-linux-thread-multi/CORE

Grafting fails to replace the latter instance of the version number,
and so lookups fail like this:

------
[...]
openat(AT_FDCWD, "/gnu/store/4sqaib7c2dfjv62ivrg9b8wa7bh226la-glibc-2.26.105-g0890d5379c/lib/libperl.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/libperl.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/gnu/store/95b2fv709zvxdrb38l2w8jd73vz5v5ff-fontconfig-2.12.6/lib/libperl.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/gnu/store/4pj266bvxiris0vbvm1j568n1smdj7m8-freetype-2.8.1/lib/libperl.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/gnu/store/6n2pn58p6l7v9icg0m3i8kp4wbn8nzl2-libxft-2.3.2/lib/libperl.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/gnu/store/7lm90jgm2y00382yqriq4yw1nvb3p7b6-libxrender-0.9.10/lib/libperl.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/gnu/store/6x1p96h51dlgpdsmrx3821ixbawbm19l-libx11-1.6.5/lib/libperl.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../tls/x86_64/x86_64/libperl.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
stat("/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../tls/x86_64/x86_64", 0x7ffd43384b10) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../tls/x86_64/libperl.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
stat("/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../tls/x86_64", 0x7ffd43384b10) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../tls/x86_64/libperl.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
stat("/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../tls/x86_64", 0x7ffd43384b10) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../tls/libperl.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
stat("/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../tls", 0x7ffd43384b10) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../x86_64/x86_64/libperl.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
stat("/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../x86_64/x86_64", 0x7ffd43384b10) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../x86_64/libperl.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
stat("/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../x86_64", 0x7ffd43384b10) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../x86_64/libperl.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
stat("/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../x86_64", 0x7ffd43384b10) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../../libperl.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
stat("/gnu/store/2ifmksc425qcysl5rkxkbv6yrgc1w9cs-gcc-5.5.0-lib/lib/gcc/x86_64-unknown-linux-gnu/5.5.0/../../..", {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0
openat(AT_FDCWD, "/gnu/store/4sqaib7c2dfjv62ivrg9b8wa7bh226la-glibc-2.26.105-g0890d5379c/lib/libperl.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
writev(2, [{iov_base="urxvt", iov_len=5}, {iov_base=": ", iov_len=2}, {iov_base="error while loading shared libra"..., iov_len=36}, {iov_base=": ", iov_len=2}, {iov_base="libperl.so", iov_len=10}, {iov_base=": ", iov_len=2}, {iov_base="cannot open shared object file", iov_len=30}, {iov_base=": ", iov_len=2}, {iov_base="No such file or directory", iov_len=25}, {iov_base="\n", iov_len=1}], 10urxvt: error while loading shared libraries: libperl.so: cannot open shared object file: No such file or directory
) = 115
exit_group(127)                         = ?
------

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#31210: Perl graft breakage

[Mark H Weaver]

Hi Leo,

Leo Famulari <leo@famulari.name> writes:

> On Sun, Apr 15, 2018 at 05:34:05PM +0200, Marius Bakke wrote:
>> * gnu/packages/perl.scm (perl-5.26.2): New public variable.
>> (perl)[replacement]: New field.
>
> I noticed on my Debian system that this broke rxvt-unicode, and it
> probably affects other packages as well.
>
> The issue is that the full path of libperl.so includes the Perl version
> number, for example:
>
> /gnu/store/...-perl-5.26.1/lib/perl5/5.26.1/x86_64-linux-thread-multi/CORE/libperl.so
>
> The RUNPATH of urxvt with the Perl graft includes this bogus path:
>
> /gnu/store/ynk3rg3i791i6k7rz2imbyj9wyfjrnrc-perl-5.26.2/lib/perl5/5.26.1/x86_64-linux-thread-multi/CORE

I suggest that we add a phase to the replacement 'perl' to install a
symlink from $out/lib/perl5/5.26.1 pointing to "5.26.2".

Would you like to try this, Leo, and push it if it solves the problem
for you?

       Mark

bug#31210: Perl graft breakage

[Leo Famulari]

[-- Attachment #1.1: Type: text/plain, Size: 610 bytes --]

On Wed, Apr 18, 2018 at 03:14:45PM -0400, Mark H Weaver wrote:
> I suggest that we add a phase to the replacement 'perl' to install a
> symlink from $out/lib/perl5/5.26.1 pointing to "5.26.2".
> 
> Would you like to try this, Leo, and push it if it solves the problem
> for you?

Thanks for your advice. The attached patch fixes the issue for me.

I would have pushed it but there is one thing I don't understand. Before
I set (replacement #f), the build would fail because the new phase was
run twice, and symlinking fails because it is not "forced" (as in `ln
-sf`). Is this expected behaviour?

[-- Attachment #1.2: 0001-gnu-Perl-Fix-a-grafting-failure-due-to-a-path-that-i.patch --]
[-- Type: text/plain, Size: 2266 bytes --]

From b49afa6cf9e18d57e67c0580a09e0ee9168218e7 Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Wed, 18 Apr 2018 16:41:39 -0400
Subject: [PATCH] gnu: Perl: Fix a grafting failure due to a path that includes
 the package version.

Fixes <https://bugs.gnu.org/31210>.

* gnu/packages/perl.scm (perl-5.26.2)[arguments]: Add a
'workaround-grafting-version-bug' phase.
---
 gnu/packages/perl.scm | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/perl.scm b/gnu/packages/perl.scm
index 34eef565b..24e055225 100644
--- a/gnu/packages/perl.scm
+++ b/gnu/packages/perl.scm
@@ -43,6 +43,7 @@
   #:use-module (guix download)
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system perl)
+  #:use-module (guix utils) ;substitute-keyword-arguments for perl-5.26.2
   #:use-module (gnu packages base)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages perl-check)
@@ -161,13 +162,27 @@
 (define-public perl-5.26.2
   (package/inherit perl
     (version "5.26.2")
+    (replacement #f)
     (source (origin
               (inherit (package-source perl))
               (uri (string-append "mirror://cpan/src/5.0/perl-"
                                   version ".tar.gz"))
               (sha256
                (base32
-                "03gpnxx1g6hvlh0v4aqx00580h787sfywp1vlvw64q2xcbm9qbsp"))))))
+                "03gpnxx1g6hvlh0v4aqx00580h787sfywp1vlvw64q2xcbm9qbsp"))))
+    (arguments
+     (substitute-keyword-arguments (package-arguments perl)
+       ((#:phases phases)
+        `(modify-phases ,phases
+           ;; The path to libperl.so includes the Perl version number, and this
+           ;; is not handled by grafting. See <https://bugs.gnu.org/31210>.
+           (add-after 'install 'workaround-grafting-version-bug
+             (lambda* (#:key outputs #:allow-other-keys)
+               (let* ((out (assoc-ref outputs "out"))
+                      (new (string-append out "/lib/perl5/5.26.2"))
+                      (old (string-append out "/lib/perl5/5.26.1")))
+                 (symlink new old)
+                 #t)))))))))
 
 (define-public perl-algorithm-c3
   (package
-- 
2.17.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#31210: Perl graft breakage

[Mark H Weaver]

Leo Famulari <leo@famulari.name> writes:

> On Wed, Apr 18, 2018 at 03:14:45PM -0400, Mark H Weaver wrote:
>> I suggest that we add a phase to the replacement 'perl' to install a
>> symlink from $out/lib/perl5/5.26.1 pointing to "5.26.2".
>> 
>> Would you like to try this, Leo, and push it if it solves the problem
>> for you?
>
> Thanks for your advice. The attached patch fixes the issue for me.
>
> I would have pushed it but there is one thing I don't understand. Before
> I set (replacement #f), the build would fail because the new phase was
> run twice, and symlinking fails because it is not "forced" (as in `ln
> -sf`). Is this expected behaviour?

The problem is that you should never use 'package/inherit' to create a
graft.  That leads to an infinite tower of grafts.  I'm surprised it
works at all, but it's definitely not right.  Use (package (inherit ...)
...) instead, and leave out the (replacement #f).

     Thanks!
       Mark

bug#31210: Perl graft breakage

[Mark H Weaver]

Hi again,

I wrote:
> The problem is that you should never use 'package/inherit' to create a
> graft.  That leads to an infinite tower of grafts.

I should explain why this is.  We cannot use (package/inherit P ...) to
define the replacement of package P, because it leads to a circular
definition.  Recall that (package/inherit P ...) applies the given
transformation to both P and its replacement.  In this case, P's
replacement is the thing we're trying to define.  See the circularity?

This would lead to an infinite recursion if not for the fact that
'replacement' is a "thunked" field, i.e. its evaluation is delayed.
Instead it leads to an infinite series of replacements, each one based
on the previous one, but with the transformation applied one more time.

In this example with 'perl', the transformation involved adding a new
phase.  Let's call that transformation 'f', so that (f perl) is the
replacement you wanted.  Since you used (package/inherit perl ...) to
define perl's replacement, it led to an infinite series of replacements:

  perl
  (f perl)
  (f (f perl))
  (f (f (f perl)))
  ...

I would have expected the grafting machinery to iterate until reaching
the end of this series of replacements.  I guess it didn't because the
(f (f perl)) build failed, since it added the phase twice.

Also note that since 'package/inherit' implicitly adds its own
'replacement' field, if you manually override 'replacement' within
'package/inherit' as you did in your first proposed patch, it expands
into a package definition with two 'replacement' overrides, like this:

  (package (inherit P)
    (replacement #f)
    ...
    (replacement ...))

Ideally, guix records would report an error in this case.

     Regards,
       Mark

bug#31210: Perl graft breakage

[Mark H Weaver]

Hi Leo,

One more thing:

Leo Famulari <leo@famulari.name> writes:
> +           (add-after 'install 'workaround-grafting-version-bug
> +             (lambda* (#:key outputs #:allow-other-keys)
> +               (let* ((out (assoc-ref outputs "out"))
> +                      (new (string-append out "/lib/perl5/5.26.2"))
> +                      (old (string-append out "/lib/perl5/5.26.1")))
> +                 (symlink new old)
> +                 #t)))))))))

It would be better for 'new' to be simply "5.26.2" instead of an
absolute file name.  If I'm not mistaken, on ext4, files (including
symlinks) smaller than 60 bytes are stored entirely within the inode,
saving both a disk block and a seek operation when accessing it.

      Mark

bug#31210: Perl graft breakage

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 947 bytes --]

On Thu, Apr 19, 2018 at 07:57:32AM -0400, Mark H Weaver wrote:
> Hi Leo,
> 
> One more thing:
> 
> Leo Famulari <leo@famulari.name> writes:
> > +           (add-after 'install 'workaround-grafting-version-bug
> > +             (lambda* (#:key outputs #:allow-other-keys)
> > +               (let* ((out (assoc-ref outputs "out"))
> > +                      (new (string-append out "/lib/perl5/5.26.2"))
> > +                      (old (string-append out "/lib/perl5/5.26.1")))
> > +                 (symlink new old)
> > +                 #t)))))))))
> 
> It would be better for 'new' to be simply "5.26.2" instead of an
> absolute file name.  If I'm not mistaken, on ext4, files (including
> symlinks) smaller than 60 bytes are stored entirely within the inode,
> saving both a disk block and a seek operation when accessing it.

Thanks for all your expert advice! I pushed a fix as
44b98b00026e62766620dbc4330a305282d61069.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#31210: Perl graft breakage

[Marius Bakke]

[-- Attachment #1.1: Type: text/plain, Size: 1046 bytes --]

Mark H Weaver <mhw@netris.org> writes:

> Hi Leo,
>
> One more thing:
>
> Leo Famulari <leo@famulari.name> writes:
>> +           (add-after 'install 'workaround-grafting-version-bug
>> +             (lambda* (#:key outputs #:allow-other-keys)
>> +               (let* ((out (assoc-ref outputs "out"))
>> +                      (new (string-append out "/lib/perl5/5.26.2"))
>> +                      (old (string-append out "/lib/perl5/5.26.1")))
>> +                 (symlink new old)
>> +                 #t)))))))))
>
> It would be better for 'new' to be simply "5.26.2" instead of an
> absolute file name.  If I'm not mistaken, on ext4, files (including
> symlinks) smaller than 60 bytes are stored entirely within the inode,
> saving both a disk block and a seek operation when accessing it.

Hello Mark and Leo,

I'm sorry for not testing this graft more thoroughly.  And thanks for
the explanation about package/inherit, I had misunderstood its purpose.

The attached patch should address all concerns.  I have also tested
urxvt with it.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-gnu-perl-Fix-grafting-error-due-to-version-number-in.patch --]
[-- Type: text/x-patch, Size: 2369 bytes --]

From 44ae580e4affc03bcfd6d27277ea78aa63021fb9 Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@fastmail.com>
Date: Thu, 19 Apr 2018 17:17:54 +0200
Subject: [PATCH] gnu: perl: Fix grafting error due to version number in Perl
 library path.

Fixes <https://bugs.gnu.org/31210>.

* gnu/packages/perl.scm (perl-5.26.2): Use (package (inherit) ...) instead
of (package/inherit ...).
[arguments]: Add phase 'workaround-grafting-version-bug'.
---
 gnu/packages/perl.scm | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/perl.scm b/gnu/packages/perl.scm
index aaf3e2e82..4cd5bf6df 100644
--- a/gnu/packages/perl.scm
+++ b/gnu/packages/perl.scm
@@ -44,6 +44,7 @@
   #:use-module (guix download)
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system perl)
+  #:use-module (guix utils) ;substitute-keyword-arguments for perl-5.26.2
   #:use-module (gnu packages base)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages freedesktop)
@@ -161,7 +162,8 @@
 ;; Fixes CVE-2018-6797, CVE-2018-6798, and CVE-2018-6913.
 ;; See <https://metacpan.org/changes/release/SHAY/perl-5.26.2>.
 (define-public perl-5.26.2
-  (package/inherit perl
+  (package
+    (inherit perl)
     (version "5.26.2")
     (source (origin
               (inherit (package-source perl))
@@ -169,7 +171,19 @@
                                   version ".tar.gz"))
               (sha256
                (base32
-                "03gpnxx1g6hvlh0v4aqx00580h787sfywp1vlvw64q2xcbm9qbsp"))))))
+                "03gpnxx1g6hvlh0v4aqx00580h787sfywp1vlvw64q2xcbm9qbsp"))))
+    (arguments
+     (substitute-keyword-arguments (package-arguments perl)
+       ((#:phases phases)
+        `(modify-phases ,phases
+           ;; The path to libperl.so includes the Perl version number, and this
+           ;; is not handled by grafting. See <https://bugs.gnu.org/31210>.
+           (add-after 'install 'workaround-grafting-version-bug
+             (lambda* (#:key outputs #:allow-other-keys)
+               (let* ((out (assoc-ref outputs "out")))
+                 (with-directory-excursion (string-append out "/lib/perl5")
+                   (symlink "5.26.2" "5.26.1")
+                   #t))))))))))
 
 (define-public perl-algorithm-c3
   (package
-- 
2.17.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#31210: Perl graft breakage

[Mark H Weaver]

My suggested approach of making compatibility symlinks didn't work.
See <https://bugs.gnu.org/31216> for details and further discussion.

      Mark