* bug#28157: “r-minimal” retains no reference to “which” @ 2017-08-20 11:16 Ricardo Wurmus 2017-08-22 8:51 ` Ludovic Courtès 0 siblings, 1 reply; 9+ messages in thread From: Ricardo Wurmus @ 2017-08-20 11:16 UTC (permalink / raw) To: 28157 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#28157: “r-minimal” retains no reference to “which” 2017-08-20 11:16 bug#28157: “r-minimal” retains no reference to “which” Ricardo Wurmus @ 2017-08-22 8:51 ` Ludovic Courtès 2017-08-22 9:19 ` Ricardo Wurmus 0 siblings, 1 reply; 9+ messages in thread From: Ludovic Courtès @ 2017-08-22 8:51 UTC (permalink / raw) To: Ricardo Wurmus; +Cc: 28157 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’. ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#28157: “r-minimal” retains no reference to “which” 2017-08-22 8:51 ` Ludovic Courtès @ 2017-08-22 9:19 ` Ricardo Wurmus 2017-08-22 9:29 ` Ricardo Wurmus 2017-08-22 9:38 ` Ludovic Courtès 0 siblings, 2 replies; 9+ messages in thread From: Ricardo Wurmus @ 2017-08-22 9:19 UTC (permalink / raw) To: Ludovic Courtès; +Cc: 28157 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#28157: “r-minimal” retains no reference to “which” 2017-08-22 9:19 ` Ricardo Wurmus @ 2017-08-22 9:29 ` Ricardo Wurmus 2017-08-22 9:38 ` Ludovic Courtès 1 sibling, 0 replies; 9+ messages in thread From: Ricardo Wurmus @ 2017-08-22 9:29 UTC (permalink / raw) To: Ludovic Courtès; +Cc: 28157 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#28157: “r-minimal” retains no reference to “which” 2017-08-22 9:19 ` Ricardo Wurmus 2017-08-22 9:29 ` Ricardo Wurmus @ 2017-08-22 9:38 ` Ludovic Courtès 2017-08-22 11:02 ` Ricardo Wurmus 1 sibling, 1 reply; 9+ messages in thread From: Ludovic Courtès @ 2017-08-22 9:38 UTC (permalink / raw) To: Ricardo Wurmus; +Cc: 28157 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’. ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#28157: “r-minimal” retains no reference to “which” 2017-08-22 9:38 ` Ludovic Courtès @ 2017-08-22 11:02 ` Ricardo Wurmus 2017-08-22 13:40 ` Ludovic Courtès 0 siblings, 1 reply; 9+ messages in thread From: Ricardo Wurmus @ 2017-08-22 11:02 UTC (permalink / raw) To: Ludovic Courtès; +Cc: 28157 [-- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* bug#28157: “r-minimal” retains no reference to “which” 2017-08-22 11:02 ` Ricardo Wurmus @ 2017-08-22 13:40 ` Ludovic Courtès 2017-08-22 14:03 ` Ricardo Wurmus 0 siblings, 1 reply; 9+ messages in thread From: Ludovic Courtès @ 2017-08-22 13:40 UTC (permalink / raw) To: Ricardo Wurmus; +Cc: 28157 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’. ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#28157: “r-minimal” retains no reference to “which” 2017-08-22 13:40 ` Ludovic Courtès @ 2017-08-22 14:03 ` Ricardo Wurmus 2017-08-22 14:33 ` Ricardo Wurmus 0 siblings, 1 reply; 9+ messages in thread From: Ricardo Wurmus @ 2017-08-22 14:03 UTC (permalink / raw) To: Ludovic Courtès; +Cc: 28157 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#28157: “r-minimal” retains no reference to “which” 2017-08-22 14:03 ` Ricardo Wurmus @ 2017-08-22 14:33 ` Ricardo Wurmus 0 siblings, 0 replies; 9+ messages in thread From: Ricardo Wurmus @ 2017-08-22 14:33 UTC (permalink / raw) To: Ludovic Courtès; +Cc: 28157-done Pushed to master with bd3a184613e20155a8b3e417f00f4d59ff0935e6. Thanks for the assistance! -- Ricardo GPG: BCA6 89B6 3655 3801 C3C6 2150 197A 5888 235F ACAC https://elephly.net ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-08-22 14:34 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-08-20 11:16 bug#28157: “r-minimal” retains no reference to “which” Ricardo Wurmus 2017-08-22 8:51 ` Ludovic Courtès 2017-08-22 9:19 ` Ricardo Wurmus 2017-08-22 9:29 ` Ricardo Wurmus 2017-08-22 9:38 ` Ludovic Courtès 2017-08-22 11:02 ` Ricardo Wurmus 2017-08-22 13:40 ` Ludovic Courtès 2017-08-22 14:03 ` Ricardo Wurmus 2017-08-22 14:33 ` Ricardo Wurmus
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/guix.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).