[bug#28773] [PATCH] Wrap bin files in the ruby build system.

[bug#28773] [PATCH] Wrap bin files in the ruby build system.

[Christopher Baines]

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

This is a bit experimental. There is some description of the motivation
behind this approach, as well as the downsides in the commit messages.

I don't know how to wrap binaries with the GEM_PATH, but somehow
excluding inputs specified as native-inputs, so I'd be very interested
if anyone has ideas about this.

Christopher Baines (3):
  ruby-build-system: Add wrap-ruby-program.
  ruby-build-system: Add a new wrap phase.
  gnu: Remove redundant wrapping from packages using ruby-build-system.

 gnu/packages/databases.scm       |  13 +---
 gnu/packages/ruby.scm            |  18 +-----
 guix/build/ruby-build-system.scm | 131 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 133 insertions(+), 29 deletions(-)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

[bug#28773] [PATCH 1/3] ruby-build-system: Add wrap-ruby-program.

[Christopher Baines]

A modified copy of wrap-program from (guix build utils). The wrap-program
procedure doesn't work well for Ruby scripts, as it breaks using the -S flag
with ruby to execute the script, as when -S is passed to ruby, it expects the
script on the PATH to use ruby in the shebang, and not bash.

Therefore, to wrap the program, but keep the shebang as ruby, wrap it with a
ruby script instead.

wrap-ruby-program uses .real/foo rather than .foo-real, as this might be
neater. This procedure also includes a call to Gem.clear_paths to make it
possible to set the GEM_PATH through this method, and for it to take effect.

* gnu/build/ruby-build-system.scm (wrap-ruby-program): New procedure.
---
 guix/build/ruby-build-system.scm | 103 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/guix/build/ruby-build-system.scm b/guix/build/ruby-build-system.scm
index c2d276627..af193b9ce 100644
--- a/guix/build/ruby-build-system.scm
+++ b/guix/build/ruby-build-system.scm
@@ -166,6 +166,109 @@ GEM-FLAGS are passed to the 'gem' invokation, if present."
                                         "Makefile")))))
            #t))))
 
+(define* (wrap-ruby-program prog #:key (gem-clear-paths #t) #:rest vars)
+  "Make a wrapper for PROG.  VARS should look like this:
+
+  '(VARIABLE DELIMITER POSITION LIST-OF-DIRECTORIES)
+
+where DELIMITER is optional.  ':' will be used if DELIMITER is not given.
+
+For example, this command:
+
+  (wrap-ruby-program \"foo\"
+                '(\"PATH\" \":\" = (\"/gnu/.../bar/bin\"))
+                '(\"CERT_PATH\" suffix (\"/gnu/.../baz/certs\"
+                                        \"/qux/certs\")))
+
+will copy 'foo' to '.real/fool' and create the file 'foo' with the following
+contents:
+
+  #!location/of/bin/ruby
+  ENV['PATH'] = \"/gnu/.../bar/bin\"
+  ENV['CERT_PATH'] = (ENV.key?('CERT_PATH') ? (ENV['CERT_PATH'] + ':') : '') + '/gnu/.../baz/certs:/qux/certs'
+  load location/of/.real/foo
+
+This is useful for scripts that expect particular programs to be in $PATH, for
+programs that expect particular gems to be in the GEM_PATH.
+
+This is preferable to wrap-program, which uses a bash script, as this prevents
+ruby scripts from being executed with @command{ruby -S ...}.
+
+If PROG has previously been wrapped by 'wrap-ruby-program', the wrapper is
+extended with definitions for VARS."
+  (define wrapped-file
+    (string-append (dirname prog) "/.real/" (basename prog)))
+
+  (define already-wrapped?
+    (file-exists? wrapped-file))
+
+  (define (last-line port)
+    ;; Return the last line read from PORT and leave PORT's cursor right
+    ;; before it.
+    (let loop ((previous-line-offset 0)
+               (previous-line "")
+               (position (seek port 0 SEEK_CUR)))
+      (match (read-line port 'concat)
+        ((? eof-object?)
+         (seek port previous-line-offset SEEK_SET)
+         previous-line)
+        ((? string? line)
+         (loop position line (+ (string-length line) position))))))
+
+  (define (export-variable lst)
+    ;; Return a string that exports an environment variable.
+    (match lst
+      ((var sep '= rest)
+       (format #f "ENV['~a'] = '~a'"
+               var (string-join rest sep)))
+      ((var sep 'prefix rest)
+       (format #f "ENV['~a'] = '~a' + (ENV.key?('~a') ? ('~a' + ENV['~a']) : '')"
+               var (string-join rest sep) var sep var))
+      ((var sep 'suffix rest)
+       (format #f "ENV['~a'] = (ENV.key?('~a') ? (ENV['~a'] + '~a') : '') + '~a'"
+               var var var sep (string-join rest sep)))
+      ((var '= rest)
+       (format #f "ENV['~a'] = '~a'"
+               var (string-join rest ":")))
+      ((var 'prefix rest)
+       (format #f "ENV['~a'] = '~a' + (ENV.key?('~a') ? (':' + ENV['~a']) : '')"
+               var (string-join rest ":") var var))
+      ((var 'suffix rest)
+       (format #f "ENV['~a'] = (ENV.key?('~a') ? (ENV['~a'] + ':') : '') + '~a'"
+               var var var (string-join rest ":")))))
+
+  (if already-wrapped?
+
+      ;; PROG is already a wrapper: add the new "export VAR=VALUE" lines just
+      ;; before the last line.
+      (let* ((port (open-file prog "r+"))
+             (last (last-line port)))
+        (for-each (lambda (var)
+                    (display (export-variable var) port)
+                    (newline port))
+                  vars)
+        (display last port)
+        (close-port port))
+
+      ;; PROG is not wrapped yet: create a shell script that sets VARS.
+      (let ((prog-tmp (string-append wrapped-file "-tmp")))
+        (mkdir-p (dirname prog-tmp))
+        (link prog wrapped-file)
+
+        (call-with-output-file prog-tmp
+          (lambda (port)
+            (format port
+                    "#!~a~%~a~%~a~%load '~a'~%"
+                    (which "ruby")
+                    (string-join (map export-variable vars) "\n")
+                    ;; This ensures that if the GEM_PATH has been changed,
+                    ;; then that change will be noticed.
+                    (if gem-clear-paths "Gem.clear_paths" "")
+                    (canonicalize-path wrapped-file))))
+
+        (chmod prog-tmp #o755)
+        (rename-file prog-tmp prog))))
+
 (define (log-file-deletion file)
   (display (string-append "deleting '" file "' for reproducibility\n")))
 
-- 
2.14.2

[bug#28773] [PATCH 2/3] ruby-build-system: Add a new wrap phase.

[Christopher Baines]

Wrap files in bin/ and sbin/ with the location of the gem itself (GEM_HOME)
and the location of any other gems in use (GEM_PATH). This ensures that the
bin files will run with the right environment when executed.

It does however mean that native-inputs will also get wrapped up in any
binaries, which is not good, as it increases the size of the closure, and
risks this code being used at runtime.

* guix/build/ruby-build-system.scm (wrap): New procedure.
  (%standard-phases): Add the wrap phase.
---
 guix/build/ruby-build-system.scm | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/guix/build/ruby-build-system.scm b/guix/build/ruby-build-system.scm
index af193b9ce..b6f0fc941 100644
--- a/guix/build/ruby-build-system.scm
+++ b/guix/build/ruby-build-system.scm
@@ -21,6 +21,7 @@
 (define-module (guix build ruby-build-system)
   #:use-module ((guix build gnu-build-system) #:prefix gnu:)
   #:use-module (guix build utils)
+  #:use-module (ice-9 ftw)
   #:use-module (ice-9 match)
   #:use-module (ice-9 popen)
   #:use-module (ice-9 regex)
@@ -269,6 +270,30 @@ extended with definitions for VARS."
         (chmod prog-tmp #o755)
         (rename-file prog-tmp prog))))
 
+(define* (wrap #:key inputs outputs #:allow-other-keys)
+  (define (list-of-files dir)
+    (map (cut string-append dir "/" <>)
+         (or (scandir dir (lambda (f)
+                            (let ((s (stat (string-append dir "/" f))))
+                              (eq? 'regular (stat:type s)))))
+             '())))
+
+  (define bindirs
+    (append-map (match-lambda
+                 ((_ . dir)
+                  (list (string-append dir "/bin")
+                        (string-append dir "/sbin"))))
+                outputs))
+
+  (let* ((out  (assoc-ref outputs "out"))
+         (var `("GEM_PATH" prefix
+                (,(getenv "GEM_HOME") ,(getenv "GEM_PATH")))))
+    (for-each (lambda (dir)
+                (let ((files (list-of-files dir)))
+                  (for-each (cut wrap-ruby-program <> var)
+                            files)))
+              bindirs)))
+
 (define (log-file-deletion file)
   (display (string-append "deleting '" file "' for reproducibility\n")))
 
@@ -280,7 +305,8 @@ extended with definitions for VARS."
     (add-after 'extract-gemspec 'replace-git-ls-files replace-git-ls-files)
     (replace 'build build)
     (replace 'check check)
-    (replace 'install install)))
+    (replace 'install install)
+    (add-after 'install 'wrap wrap)))
 
 (define* (ruby-build #:key inputs (phases %standard-phases)
                      #:allow-other-keys #:rest args)
-- 
2.14.2

[bug#28773] [PATCH 3/3] gnu: Remove redundant wrapping from packages using ruby-build-system.

[Christopher Baines]

Now that the build system does wrapping automatically, it can be removed from
the packages that do it manually.

* gnu/packages/databases.scm (es-dump-restore)[arguments]: Remove #:phases.
* gnu/packages/ruby.scm (ruby-redcloth)[arguments]: Remove #:phases.
  (ruby-httpclient)[arguments]: Remove 'wrap-bin-httpclient from the modified
  phases.
---
 gnu/packages/databases.scm | 13 +------------
 gnu/packages/ruby.scm      | 18 ++----------------
 2 files changed, 3 insertions(+), 28 deletions(-)

diff --git a/gnu/packages/databases.scm b/gnu/packages/databases.scm
index 62f940e90..69bb9fd7d 100644
--- a/gnu/packages/databases.scm
+++ b/gnu/packages/databases.scm
@@ -273,18 +273,7 @@ SQL, Key/Value, XML/XQuery or Java Object storage for their data model.")
          "020yk7f1hw48clmf5501z3xv9shsdchyymcv0y2cci2c1xvr1mim"))))
     (build-system ruby-build-system)
     (arguments
-     '(#:tests? #f ;; No testsuite.
-       #:phases
-       (modify-phases %standard-phases
-         (add-after 'install 'wrap-bin-es_dump_restore
-           (lambda* (#:key outputs #:allow-other-keys)
-             (wrap-program (string-append (assoc-ref outputs "out")
-                                          "/bin/es_dump_restore")
-               `("GEM_PATH" ":" prefix (,(string-append
-                                          (getenv "GEM_PATH")
-                                          ":"
-                                          (getenv "GEM_HOME")))))
-             #t)))))
+     '(#:tests? #f)) ;; No testsuite.
     (propagated-inputs
      `(("ruby-httpclient" ,ruby-httpclient)
        ("ruby-multi-json" ,ruby-multi-json)
diff --git a/gnu/packages/ruby.scm b/gnu/packages/ruby.scm
index f2333dbcb..83e106d3c 100644
--- a/gnu/packages/ruby.scm
+++ b/gnu/packages/ruby.scm
@@ -2896,15 +2896,7 @@ alternative to Marshal for Object serialization. ")
          ;; existing gemspec.
          (replace 'build
           (lambda _
-            (zero? (system* "gem" "build" "redcloth.gemspec"))))
-         ;; Make sure that the "redcloth" executable finds required Ruby
-         ;; libraries.
-         (add-after 'install 'wrap-bin-redcloth
-          (lambda* (#:key outputs #:allow-other-keys)
-            (wrap-program (string-append (assoc-ref outputs "out")
-                                         "/bin/redcloth")
-              `("GEM_HOME" ":" prefix (,(getenv "GEM_HOME"))))
-            #t)))))
+            (zero? (system* "gem" "build" "redcloth.gemspec")))))))
     (native-inputs
      `(("bundler" ,bundler)
        ("ruby-diff-lcs" ,ruby-diff-lcs)
@@ -3752,13 +3744,7 @@ It has built-in support for the legacy @code{cookies.txt} and
                   (system* "ruby"
                            "-Ilib"
                            "test/runner.rb"))
-                 #t)))
-         (add-after 'install 'wrap-bin-httpclient
-           (lambda* (#:key outputs #:allow-other-keys)
-             (wrap-program (string-append (assoc-ref outputs "out")
-                                          "/bin/httpclient")
-               `("GEM_HOME" ":" prefix (,(getenv "GEM_HOME"))))
-             #t)))))
+                 #t))))))
     (native-inputs
      `(("ruby-rack" ,ruby-rack)))
     (synopsis
-- 
2.14.2

[bug#28773] [PATCH] Wrap bin files in the ruby build system.

[Ludovic Courtès]

Hi Chris,

Christopher Baines <mail@cbaines.net> skribis:

> This is a bit experimental. There is some description of the motivation
> behind this approach, as well as the downsides in the commit messages.
>
> I don't know how to wrap binaries with the GEM_PATH, but somehow
> excluding inputs specified as native-inputs, so I'd be very interested
> if anyone has ideas about this.
>
> Christopher Baines (3):
>   ruby-build-system: Add wrap-ruby-program.
>   ruby-build-system: Add a new wrap phase.
>   gnu: Remove redundant wrapping from packages using ruby-build-system.

Cc’ing Ben who is another Serious Ruby User I think.  :-)

Ludo’.

[bug#28773] [PATCH] Wrap bin files in the ruby build system.

[Ben Woodcroft]

Hi,


On 16/10/17 23:19, Ludovic Courtès wrote:
> Hi Chris,
>
> Christopher Baines <mail@cbaines.net> skribis:
>
>> This is a bit experimental. There is some description of the motivation
>> behind this approach, as well as the downsides in the commit messages.
>>
>> I don't know how to wrap binaries with the GEM_PATH, but somehow
>> excluding inputs specified as native-inputs, so I'd be very interested
>> if anyone has ideas about this.
>>
>> Christopher Baines (3):
>>    ruby-build-system: Add wrap-ruby-program.
>>    ruby-build-system: Add a new wrap phase.
>>    gnu: Remove redundant wrapping from packages using ruby-build-system.
> Cc’ing Ben who is another Serious Ruby User I think.  :-)

Maybe sometimes. I'll try to get a review back to you in the next week 
or so.
ben

[bug#28773] [PATCH] Wrap bin files in the ruby build system.

[Ludovic Courtès]

Hello!

Ben Woodcroft <b.woodcroft@uq.edu.au> skribis:

> On 16/10/17 23:19, Ludovic Courtès wrote:
>> Hi Chris,
>>
>> Christopher Baines <mail@cbaines.net> skribis:
>>
>>> This is a bit experimental. There is some description of the motivation
>>> behind this approach, as well as the downsides in the commit messages.
>>>
>>> I don't know how to wrap binaries with the GEM_PATH, but somehow
>>> excluding inputs specified as native-inputs, so I'd be very interested
>>> if anyone has ideas about this.
>>>
>>> Christopher Baines (3):
>>>    ruby-build-system: Add wrap-ruby-program.
>>>    ruby-build-system: Add a new wrap phase.
>>>    gnu: Remove redundant wrapping from packages using ruby-build-system.
>> Cc’ing Ben who is another Serious Ruby User I think.  :-)
>
> Maybe sometimes. I'll try to get a review back to you in the next week
> or so.

Ahem.  :-)

Chris, if you’re confident, I’d say you can go ahead.  It does sound
like an improvement so it’d be sad to lose it.

Ludo’.

[bug#28773] [PATCH] Wrap bin files in the ruby build system.

[Ben Woodcroft]

Hello!


On 12/01/18 07:47, Ludovic Courtès wrote:
> Hello!
>
> Ben Woodcroft <b.woodcroft@uq.edu.au> skribis:
>
>> On 16/10/17 23:19, Ludovic Courtès wrote:
>>> Hi Chris,
>>>
>>> Christopher Baines <mail@cbaines.net> skribis:
>>>
>>>> This is a bit experimental. There is some description of the motivation
>>>> behind this approach, as well as the downsides in the commit messages.
>>>>
>>>> I don't know how to wrap binaries with the GEM_PATH, but somehow
>>>> excluding inputs specified as native-inputs, so I'd be very interested
>>>> if anyone has ideas about this.
>>>>
>>>> Christopher Baines (3):
>>>>     ruby-build-system: Add wrap-ruby-program.
>>>>     ruby-build-system: Add a new wrap phase.
>>>>     gnu: Remove redundant wrapping from packages using ruby-build-system.
>>> Cc’ing Ben who is another Serious Ruby User I think.  :-)
>> Maybe sometimes. I'll try to get a review back to you in the next week
>> or so.
> Ahem.  :-)
>
> Chris, if you’re confident, I’d say you can go ahead.  It does sound
> like an improvement so it’d be sad to lose it.
Ah yes, my apologies. Yes, this series LGTM, and it worked fine when I 
applied it in local packages too.

Thanks.
ben

[bug#28773] [PATCH] Wrap bin files in the ruby build system.

[Christopher Baines]

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


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

> Hello!
>
> Ben Woodcroft <b.woodcroft@uq.edu.au> skribis:
>
>> On 16/10/17 23:19, Ludovic Courtès wrote:
>>> Hi Chris,
>>>
>>> Christopher Baines <mail@cbaines.net> skribis:
>>>
>>>> This is a bit experimental. There is some description of the motivation
>>>> behind this approach, as well as the downsides in the commit messages.
>>>>
>>>> I don't know how to wrap binaries with the GEM_PATH, but somehow
>>>> excluding inputs specified as native-inputs, so I'd be very interested
>>>> if anyone has ideas about this.
>>>>
>>>> Christopher Baines (3):
>>>>    ruby-build-system: Add wrap-ruby-program.
>>>>    ruby-build-system: Add a new wrap phase.
>>>>    gnu: Remove redundant wrapping from packages using ruby-build-system.
>>> Cc’ing Ben who is another Serious Ruby User I think.  :-)
>>
>> Maybe sometimes. I'll try to get a review back to you in the next week
>> or so.
>
> Ahem.  :-)
>
> Chris, if you’re confident, I’d say you can go ahead.  It does sound
> like an improvement so it’d be sad to lose it.

Great, I've now pushed this, along with the other ruby build system
change.

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

[bug#28773] [PATCH] Wrap bin files in the ruby build system.

[Christopher Baines]

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


Ben Woodcroft <b.woodcroft@uq.edu.au> writes:

> Hello!
>
>
> On 12/01/18 07:47, Ludovic Courtès wrote:
>> Hello!
>>
>> Ben Woodcroft <b.woodcroft@uq.edu.au> skribis:
>>
>>> On 16/10/17 23:19, Ludovic Courtès wrote:
>>>> Hi Chris,
>>>>
>>>> Christopher Baines <mail@cbaines.net> skribis:
>>>>
>>>>> This is a bit experimental. There is some description of the motivation
>>>>> behind this approach, as well as the downsides in the commit messages.
>>>>>
>>>>> I don't know how to wrap binaries with the GEM_PATH, but somehow
>>>>> excluding inputs specified as native-inputs, so I'd be very interested
>>>>> if anyone has ideas about this.
>>>>>
>>>>> Christopher Baines (3):
>>>>>     ruby-build-system: Add wrap-ruby-program.
>>>>>     ruby-build-system: Add a new wrap phase.
>>>>>     gnu: Remove redundant wrapping from packages using ruby-build-system.
>>>> Cc’ing Ben who is another Serious Ruby User I think.  :-)
>>> Maybe sometimes. I'll try to get a review back to you in the next week
>>> or so.
>> Ahem.  :-)
>>
>> Chris, if you’re confident, I’d say you can go ahead.  It does sound
>> like an improvement so it’d be sad to lose it.
> Ah yes, my apologies. Yes, this series LGTM, and it worked fine when I 
> applied it in local packages too.

Super, thanks for taking a look Ben :)

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

bug#28773: [PATCH] Specify native search path for all ruby packages

[Christopher Baines]

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


Closing, as I've now pushed this patch.

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