bug#28157: “r-minimal” retains no reference to “which”

bug#28157: “r-minimal” retains no reference to “which”

[Ricardo Wurmus]

R provides a function “Sys.which”, which embeds the full path to “which”
at configure time.  The path it embeds is that of “which” *before*
grafts.

The final package does not retain any reference to the “which” package,
however.  What’s worse: this cannot be fixed by adding “which” to the
environment, as “Sys.which” holds a reference to the ungrafted “which”
package.

This is a problem in containers, where the ungrafted “which” will not be
available even when the “which” package is added to the container.

It may also be a security problem, because R references an ungrafted
package.

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

bug#28157: “r-minimal” retains no reference to “which”

[Ludovic Courtès]

Ricardo Wurmus <rekado@elephly.net> skribis:

> R provides a function “Sys.which”, which embeds the full path to “which”
> at configure time.  The path it embeds is that of “which” *before*
> grafts.

Where’s that reference?  I couldn’t find it:

--8<---------------cut here---------------start------------->8---
$ guix build r-minimal

[...]

@ build-succeeded /gnu/store/xz3jhg34z1znlzkz45pva2x0ik96w3qh-r-minimal-3.4.0.drv -
/gnu/store/jzfg7j6wbp28z5zfdh16wyhkqy3azm2z-r-minimal-3.4.0
$ guix gc --references /gnu/store/jzfg7j6wbp28z5zfdh16wyhkqy3azm2z-r-minimal-3.4.0|grep which
$ guix build r-minimal --no-grafts
/gnu/store/9xmz92jgmgalf4i07fdiddn658zlccpw-r-minimal-3.4.0
$ guix gc --references /gnu/store/9xmz92jgmgalf4i07fdiddn658zlccpw-r-minimal-3.4.0 |grep which
$ guix build which
/gnu/store/xaiq6waavhfrfhxjb35whzc6y4617nzz-which-2.21
$ grep -r -e -which-2.21 /gnu/store/9xmz92jgmgalf4i07fdiddn658zlccpw-r-minimal-3.4.0
$ grep -r which-2.21 /gnu/store/9xmz92jgmgalf4i07fdiddn658zlccpw-r-minimal-3.4.0
$ grep -r bin/which /gnu/store/9xmz92jgmgalf4i07fdiddn658zlccpw-r-minimal-3.4.0
--8<---------------cut here---------------end--------------->8---

Normally the grafting code replaces all the store references, unless the
reference is somehow obfuscated (which would be surprising here.)

Ludo’.

bug#28157: “r-minimal” retains no reference to “which”

[Ricardo Wurmus]

Ludovic Courtès <ludo@gnu.org> writes:

> Ricardo Wurmus <rekado@elephly.net> skribis:
>
>> R provides a function “Sys.which”, which embeds the full path to “which”
>> at configure time.  The path it embeds is that of “which” *before*
>> grafts.
>
> Where’s that reference?  I couldn’t find it:

You can see the reference (before commit
a8cd352304807ef60d06c35da07c5456f036688c) within an R session.  If you
type “Sys.which” the code for “Sys.which” will be printed.

    $ /gnu/store/hzc1fnrjv8ys399glbzmhds21fm9zzva-r-minimal-3.4.0/bin/R -e 'Sys.which' | grep /gnu/store
    which <- "/gnu/store/ppyczjc1figwv6yb6brg938y49856avb-which-2.21/bin/which"

(The reference is the same as “guix build --no-grafts which”.)

> Normally the grafting code replaces all the store references, unless the
> reference is somehow obfuscated (which would be surprising here.)

Right.  In the case of R, the environment for a package is serialized to
a possibly compressed data file (.Rdb) with an index (.Rdx).
“Sys.which” is part of the “base” package and I cannot find a plain text
reference to “bin/which” in the binaries for “base”.

I don’t know if we can (or should) disable compression for Rdb files.

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

bug#28157: “r-minimal” retains no reference to “which”

[Ricardo Wurmus]

Ricardo Wurmus <rekado@elephly.net> writes:

> Ludovic Courtès <ludo@gnu.org> writes:
[…]
>> Normally the grafting code replaces all the store references, unless the
>> reference is somehow obfuscated (which would be surprising here.)
>
> Right.  In the case of R, the environment for a package is serialized to
> a possibly compressed data file (.Rdb) with an index (.Rdx).
> “Sys.which” is part of the “base” package and I cannot find a plain text
> reference to “bin/which” in the binaries for “base”.
>
> I don’t know if we can (or should) disable compression for Rdb files.

If we need to do that I think it should happen in
“src/library/base/makebasedb.R”.  This file is used to create the base
library and it calls saveRDS with “compressed = TRUE” by default.

-- 
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

bug#28157: “r-minimal” retains no reference to “which”

[Ludovic Courtès]

Ricardo Wurmus <rekado@elephly.net> skribis:

> Right.  In the case of R, the environment for a package is serialized to
> a possibly compressed data file (.Rdb) with an index (.Rdx).
> “Sys.which” is part of the “base” package and I cannot find a plain text
> reference to “bin/which” in the binaries for “base”.

Doh!  That’s a problem.  The GC won’t detect those references either,
which can lead to early-deletion problems.

> I don’t know if we can (or should) disable compression for Rdb files.

We’ll probably have to disable it.  (According to ‘file’ it’s no a
standard compression format like gzip; maybe raw zlib without gzip
headers?)

Ludo’.

bug#28157: “r-minimal” retains no reference to “which”

[Ricardo Wurmus]

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


Ludovic Courtès <ludo@gnu.org> writes:

> Ricardo Wurmus <rekado@elephly.net> skribis:
>
>> Right.  In the case of R, the environment for a package is serialized to
>> a possibly compressed data file (.Rdb) with an index (.Rdx).
>> “Sys.which” is part of the “base” package and I cannot find a plain text
>> reference to “bin/which” in the binaries for “base”.
>
> Doh!  That’s a problem.  The GC won’t detect those references either,
> which can lead to early-deletion problems.
>
>> I don’t know if we can (or should) disable compression for Rdb files.
>
> We’ll probably have to disable it.  (According to ‘file’ it’s no a
> standard compression format like gzip; maybe raw zlib without gzip
> headers?)

Here’s a patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-r-minimal-Do-not-compress-serialized-files.patch --]
[-- Type: text/x-patch, Size: 1853 bytes --]

From bbacb223cbd6f1ba0ca77eda9d168e325537e3f3 Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Tue, 22 Aug 2017 12:59:48 +0200
Subject: [PATCH] gnu: r-minimal: Do not compress serialized files.

* gnu/packages/statistics.scm (r-minimal)[arguments]: Replace build phase
"patch-which" with "do-not-compress-serialized-files".
[propagated-inputs]: Move "which" from here...
[inputs]: ...to here.
---
 gnu/packages/statistics.scm | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/gnu/packages/statistics.scm b/gnu/packages/statistics.scm
index dc7491acd..676752e03 100644
--- a/gnu/packages/statistics.scm
+++ b/gnu/packages/statistics.scm
@@ -127,10 +127,10 @@ be output in text, PostScript, PDF or HTML.")
        #:phases
        (modify-phases %standard-phases
          ;; FIXME: see bug #28157.
-         (add-before 'configure 'patch-which
+         (add-before 'configure 'do-not-compress-serialized-files
            (lambda* (#:key inputs #:allow-other-keys)
-             (substitute* "src/library/base/R/unix/system.unix.R"
-               (("@WHICH@") "which"))
+             (substitute* "src/library/base/makebasedb.R"
+               (("compress = TRUE") "compress = FALSE"))
              #t))
          (add-before 'configure 'patch-uname
            (lambda* (#:key inputs #:allow-other-keys)
@@ -250,10 +250,8 @@ be output in text, PostScript, PDF or HTML.")
        ("libxt" ,libxt)
        ("pcre" ,pcre)
        ("readline" ,readline)
+       ("which" ,which)
        ("zlib" ,zlib)))
-    ;; FIXME: By default Sys.which embeds a reference to "which", but this
-    ;; reference is not detected by Guix (see bug #28157).
-    (propagated-inputs `(("which" ,which)))
     (native-search-paths
      (list (search-path-specification
             (variable "R_LIBS_SITE")
-- 
2.14.1


[-- Attachment #3: Type: text/plain, Size: 286 bytes --]


I have built r-minimal with it and confirmed that “which” is retained as
a reference.  I don’t know if this causes any other problems down the
road, but I think it should not be a problem.

-- 
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

bug#28157: “r-minimal” retains no reference to “which”

[Ludovic Courtès]

Ricardo Wurmus <rekado@elephly.net> skribis:

> Here’s a patch:

That was fast.  :-)

> From bbacb223cbd6f1ba0ca77eda9d168e325537e3f3 Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Tue, 22 Aug 2017 12:59:48 +0200
> Subject: [PATCH] gnu: r-minimal: Do not compress serialized files.
>
> * gnu/packages/statistics.scm (r-minimal)[arguments]: Replace build phase
> "patch-which" with "do-not-compress-serialized-files".
> [propagated-inputs]: Move "which" from here...
> [inputs]: ...to here.

[...]

>         (modify-phases %standard-phases
>           ;; FIXME: see bug #28157.
> -         (add-before 'configure 'patch-which
> +         (add-before 'configure 'do-not-compress-serialized-files
>             (lambda* (#:key inputs #:allow-other-keys)
> -             (substitute* "src/library/base/R/unix/system.unix.R"
> -               (("@WHICH@") "which"))

Shouldn’t we keep the ‘patch-which’ phase?

> +             (substitute* "src/library/base/makebasedb.R"
> +               (("compress = TRUE") "compress = FALSE"))

Perhaps move the comment about this bug right above this, so we know why
we don’t compress.  Otherwise LGTM!

Any idea how much extra storage this incurs on disk?

Thanks!

Ludo’.

bug#28157: “r-minimal” retains no reference to “which”

[Ricardo Wurmus]

Ludovic Courtès <ludo@gnu.org> writes:

> Ricardo Wurmus <rekado@elephly.net> skribis:
>
>> Here’s a patch:
>
> That was fast.  :-)
>
>> From bbacb223cbd6f1ba0ca77eda9d168e325537e3f3 Mon Sep 17 00:00:00 2001
>> From: Ricardo Wurmus <rekado@elephly.net>
>> Date: Tue, 22 Aug 2017 12:59:48 +0200
>> Subject: [PATCH] gnu: r-minimal: Do not compress serialized files.
>>
>> * gnu/packages/statistics.scm (r-minimal)[arguments]: Replace build phase
>> "patch-which" with "do-not-compress-serialized-files".
>> [propagated-inputs]: Move "which" from here...
>> [inputs]: ...to here.
>
> [...]
>
>>         (modify-phases %standard-phases
>>           ;; FIXME: see bug #28157.
>> -         (add-before 'configure 'patch-which
>> +         (add-before 'configure 'do-not-compress-serialized-files
>>             (lambda* (#:key inputs #:allow-other-keys)
>> -             (substitute* "src/library/base/R/unix/system.unix.R"
>> -               (("@WHICH@") "which"))
>
> Shouldn’t we keep the ‘patch-which’ phase?

I don’t think so.  I only added this to work around the bug.

By default it does the right thing and embeds the full path to “which”
(at configure time) in the library.  Since the binaries are now
uncompressed, the references are visible to Guix, so we should keep the
reference instead of just a dangling pointer to “which”, in my opinion.

>> +             (substitute* "src/library/base/makebasedb.R"
>> +               (("compress = TRUE") "compress = FALSE"))
>
> Perhaps move the comment about this bug right above this, so we know why
> we don’t compress.  Otherwise LGTM!

Okay!

> Any idea how much extra storage this incurs on disk?

The previous “r-minimal”:
total: 607.6 MiB

The new one:
total: 611.2 MiB

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

bug#28157: “r-minimal” retains no reference to “which”

[Ricardo Wurmus]

Pushed to master with bd3a184613e20155a8b3e417f00f4d59ff0935e6.

Thanks for the assistance!

-- 
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net