Re: [PATCH] Update Ruby to 2.3.0 (was Re: Freezing core-updates soon)

[Ben Woodcroft <b.woodcroft@uq.edu.au>]

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



On 06/01/16 08:24, Ludovic Courtès wrote:
> Ben Woodcroft <b.woodcroft@uq.edu.au> skribis:
>
>> I had some trouble downloading dependencies because of inability to
>> download sources e.g. http://graphviz.org/ seems to be down. Ruby
>> seems to build though, can we just review as normal and then push to
>> core-updates?
>>
>> I did rebuild all the packages in ruby.scm, and out of these about 1/3
>> of packages failed to build, because at least these three packages'
>> tests fail
>> * yard
>> * power_assert
>> * minitest_tu_shim
>>
>> The first two presumably will be fixed upstream soon, but the third
>> hasn't seen a release since Aug 2013, and the homepage listed on
>> rubygems is dead. I guess we just have to encourage the only package
>> we have packaged that uses tu_shim (term-ansicolor) to stop relying on
>> it.
> OK, thanks for testing this thoroughly.  It would be good to fix these 3
> packages sometime before core-updates is merged.
In the attached patches I've disabled the failing test in both yard and 
power_assert. There's no fix for yard in upstream. For power_assert 
there is a fix, but unfortunately it seems Ruby packages cannot be 
patched in the usual way because .gem files are archives of archives so 
the unpacking/patch/repacking fails. I guess we fix that in future.

Turns out fixing minitest_tu_shim (and others) was easier than I thought 
as the problem was some hardcoded ruby 2.2 paths in package definitions. 
I created a new procedure to remove the hard-coding so the update to 2.4 
will work too, WDYT?

After all that, everything in gnu/packages/ruby.scm builds.
>> +       (modify-phases %standard-phases
>> +         (add-before 'configure 'replace-bin-sh
>> +           (lambda _
>> +             (substitute* '("Makefile.in"
>> +                            "ext/pty/pty.c"
>> +                            "io.c"
>> +                            "lib/mkmf.rb"
>> +                            "process.c"
>> +                            "test/rubygems/test_gem_ext_configure_builder.rb"
>> +                            "test/rdoc/test_rdoc_parser.rb"
>> +                            "test/ruby/test_rubyoptions.rb"
>> +                            "test/ruby/test_process.rb"
>> +                            "test/ruby/test_system.rb"
>> +                            "tool/rbinstall.rb")
>> +               (("/bin/sh") (which "sh")))
>> +             (substitute* "ext/fiddle/libffi-3.2.1/configure"
>> +               (("SHELL = /bin/sh")
>> +                (string-append "SHELL = " (which "sh"))))
>> +             #t)))))
>> +    (inputs
>> +     `(("readline" ,readline)
>> +       ("openssl" ,openssl)
>> +       ("libffi" ,libffi)
>> +       ("gdbm" ,gdbm)
>> +       ("zlib" ,zlib)))
>> +    (native-search-paths
>> +     (list (search-path-specification
>> +            (variable "GEM_PATH")
>> +            (files (list (string-append "lib/ruby/gems/"
>> +                                        (version-major+minor version)
>> +                                        ".0"))))))
> This seems to be shared with 2.2.  Is it possible to avoid duplication
> using something like:
>
>    (define ruby
>      (package
>        (inherit ruby-2.2)
>        (version "2.3.4")
>        ;; …
>        ))
>
> ?
[..]

Yes, I did this but forgot to delete some/most duplication. Now after 
the final patch ruby-2.2 inherits from ruby (not vice-versa as you 
suggest) as this was the previous convention in ruby.scm. I checked that 
the hashes in the store names for ruby 2.2, 2.1 and 1.8 did not change 
after applying the patch.

Better? Thanks for the review.
ben

[-- Attachment #2: 0001-gnu-ruby-power-assert-Disable-failing-test.patch --]
[-- Type: text/x-patch, Size: 1376 bytes --]

From 910c98d233a271bf4ee74ee9edf4c42fb090afd5 Mon Sep 17 00:00:00 2001
From: Ben Woodcroft <donttrustben@gmail.com>
Date: Wed, 6 Jan 2016 21:23:15 +1000
Subject: [PATCH 1/4] gnu: ruby-power-assert: Disable failing test.

* gnu/packages/ruby.scm (ruby-power-assert)[arguments]: Disable test which
fails on Ruby 2.3.0.
---
 gnu/packages/ruby.scm | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/gnu/packages/ruby.scm b/gnu/packages/ruby.scm
index 47ac392..9706ec7 100644
--- a/gnu/packages/ruby.scm
+++ b/gnu/packages/ruby.scm
@@ -1067,6 +1067,18 @@ using Net::HTTP, supporting reconnection and retry according to RFC 2616.")
                (base32
                 "0gbj379jhnff8rbb6m3kzdm282szjz1a021xzxa38d1bnswj2jx3"))))
     (build-system ruby-build-system)
+    (arguments
+     '(#:phases
+       (modify-phases %standard-phases
+         (add-before 'check 'patch-test
+           ;; Disable test that fails on Ruby 2.3.  This has been fixed
+           ;; upstream but the fix has not been released.
+           (lambda _
+             (substitute*
+                 "test/test_power_assert.rb"
+               (("def test_inspection_failure")
+                "def dont_test_inspection_failure"))
+             #t)))))
     (native-inputs
      `(("bundler" ,bundler)))
     (synopsis "Assert library with descriptive assertion messages")
-- 
2.6.3


[-- Attachment #3: 0002-gnu-ruby-yard-Disable-failing-test.patch --]
[-- Type: text/x-patch, Size: 1577 bytes --]

From 207d2fa7769a4fea5a3a32a7227b2c7a2e0293bb Mon Sep 17 00:00:00 2001
From: Ben Woodcroft <donttrustben@gmail.com>
Date: Wed, 6 Jan 2016 21:57:44 +1000
Subject: [PATCH 2/4] gnu: ruby-yard: Disable failing test.

* gnu/packages/ruby.scm (ruby-yard)[arguments]: Disable test which fails on
Ruby 2.3.0.
---
 gnu/packages/ruby.scm | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/ruby.scm b/gnu/packages/ruby.scm
index 9706ec7..a695ed8 100644
--- a/gnu/packages/ruby.scm
+++ b/gnu/packages/ruby.scm
@@ -2788,9 +2788,17 @@ features such as filtering and fine grained logging.")
      `(#:test-target "specs"
        #:phases
        (modify-phases %standard-phases
-         (add-before 'check 'set-HOME
-          ;; $HOME needs to be set to somewhere writeable for tests to run
-          (lambda _ (setenv "HOME" "/tmp") #t)))))
+         (add-before 'check 'set-HOME-and-disable-failing-test
+           (lambda _
+             ;; $HOME needs to be set to somewhere writeable for tests to run
+             (setenv "HOME" "/tmp")
+             ;; Disable tests which fails on Ruby 2.3.  See
+             ;; https://github.com/lsegal/yard/issues/927
+             (substitute* "spec/parser/ruby/ruby_parser_spec.rb"
+               (("comment.type.should == :comment") "")
+               (("comment.docstring_hash_flag.should be_true") "")
+               (("comment.docstring.strip.should == .*") ""))
+             #t)))))
     (native-inputs
      `(("ruby-rspec" ,ruby-rspec-2)
        ("ruby-rack" ,ruby-rack)))
-- 
2.6.3


[-- Attachment #4: 0003-ruby-Abstract-out-path-to-GEM_HOME.patch --]
[-- Type: text/x-patch, Size: 8375 bytes --]

From 27fea9884bae4aadb4bc6981c996ddbcf54adee0 Mon Sep 17 00:00:00 2001
From: Ben Woodcroft <donttrustben@gmail.com>
Date: Fri, 8 Jan 2016 17:29:39 +1000
Subject: [PATCH 3/4] ruby: Abstract out path to GEM_HOME.

Previously paths to the GEM_HOME of certain Ruby packages were
hard-coded, so packages failed to build when Ruby was updated to 2.3.0.

* guix/build/ruby-build-system.scm (gem-home): New procedure.
* gnu/packages/ruby.scm (ruby-metaclass, ruby-instantiator,
ruby-introspection, ruby-mocha, ruby-minitest-tu-shim): Use it.
---
 gnu/packages/ruby.scm            | 92 +++++++++++++++++++++++-----------------
 guix/build/ruby-build-system.scm | 13 +++++-
 2 files changed, 65 insertions(+), 40 deletions(-)

diff --git a/gnu/packages/ruby.scm b/gnu/packages/ruby.scm
index a695ed8..71c4d05 100644
--- a/gnu/packages/ruby.scm
+++ b/gnu/packages/ruby.scm
@@ -1224,15 +1224,18 @@ It allows writing tests, checking results and automated testing in Ruby.")
      `(#:phases
        (modify-phases %standard-phases
          (add-after 'unpack 'add-test-unit-to-search-path
-          (lambda* (#:key inputs #:allow-other-keys)
-            (substitute* "Rakefile"
-              (("t\\.libs << \"test\"" line)
-               (string-append line "; t.libs << \""
-                              (assoc-ref inputs "ruby-test-unit")
-                              "/lib/ruby/gems/2.2.0/gems/test-unit-"
-                              ,(package-version ruby-test-unit)
-                              "/lib\"")))
-            #t)))))
+           (lambda* (#:key inputs #:allow-other-keys)
+             (let* ((test-unit (assoc-ref inputs "ruby-test-unit"))
+                    (test-unit-home  (gem-home test-unit
+                                               ,(package-version ruby))))
+               (substitute* "Rakefile"
+                 (("t\\.libs << \"test\"" line)
+                  (string-append line "; t.libs << \""
+                                 test-unit-home
+                                 "/gems/test-unit-"
+                                 ,(package-version ruby-test-unit)
+                                 "/lib\""))))
+             #t)))))
     (native-inputs
      `(("bundler" ,bundler)
        ("ruby-test-unit" ,ruby-test-unit)))
@@ -1286,15 +1289,17 @@ as a base class when writing classes that depend upon
      `(#:phases
        (modify-phases %standard-phases
          (add-after 'unpack 'add-test-unit-to-search-path
-          (lambda* (#:key inputs #:allow-other-keys)
-            (substitute* "Rakefile"
-              (("t\\.libs << \"test\"" line)
-               (string-append line "; t.libs << \""
-                              (assoc-ref inputs "ruby-test-unit")
-                              "/lib/ruby/gems/2.2.0/gems/test-unit-"
-                              ,(package-version ruby-test-unit)
-                              "/lib\"")))
-            #t)))))
+           (lambda* (#:key inputs #:allow-other-keys)
+             (let* ((test-unit (assoc-ref inputs "ruby-test-unit"))
+                    (test-unit-home  (gem-home test-unit ,(package-version ruby))))
+               (substitute* "Rakefile"
+                 (("t\\.libs << \"test\"" line)
+                  (string-append line "; t.libs << \""
+                                 test-unit-home
+                                 "/gems/test-unit-"
+                                 ,(package-version ruby-test-unit)
+                                 "/lib\""))))
+             #t)))))
     (propagated-inputs
      `(("ruby-blankslate" ,ruby-blankslate)))
     (native-inputs
@@ -1323,13 +1328,15 @@ knowing anything about the constructor.")
        (modify-phases %standard-phases
          (add-after 'unpack 'add-test-unit-to-search-path
           (lambda* (#:key inputs #:allow-other-keys)
-            (substitute* "Rakefile"
-              (("t\\.libs << \"test\"" line)
-               (string-append line "; t.libs << \""
-                              (assoc-ref inputs "ruby-test-unit")
-                              "/lib/ruby/gems/2.2.0/gems/test-unit-"
-                              ,(package-version ruby-test-unit)
-                              "/lib\"")))
+            (let* ((test-unit (assoc-ref inputs "ruby-test-unit"))
+                   (test-unit-home  (gem-home test-unit ,(package-version ruby))))
+              (substitute* "Rakefile"
+                (("t\\.libs << \"test\"" line)
+                 (string-append line "; t.libs << \""
+                                test-unit-home
+                                "/gems/test-unit-"
+                                ,(package-version ruby-test-unit)
+                                "/lib\""))))
             #t)))))
     (propagated-inputs
      `(("ruby-instantiator" ,ruby-instantiator)
@@ -1393,13 +1400,16 @@ conversion to (X)HTML.")
        (modify-phases %standard-phases
          (add-after 'unpack 'add-test-unit-to-search-path
           (lambda* (#:key inputs #:allow-other-keys)
-            (substitute* "Rakefile"
-              (("t\\.libs << 'test'" line)
-               (string-append line "; t.libs << \""
-                              (assoc-ref inputs "ruby-test-unit")
-                              "/lib/ruby/gems/2.2.0/gems/test-unit-"
-                              ,(package-version ruby-test-unit)
-                              "/lib\"")))
+            (let* ((test-unit (assoc-ref inputs "ruby-test-unit"))
+                   (test-unit-home  (gem-home test-unit
+                                             ,(package-version ruby))))
+              (substitute* "Rakefile"
+                (("t\\.libs << 'test'" line)
+                 (string-append line "; t.libs << \""
+                                test-unit-home
+                                "/gems/test-unit-"
+                                ,(package-version ruby-test-unit)
+                                "/lib\""))))
             #t))
          (add-before 'check 'use-latest-redcarpet
           (lambda _
@@ -2059,13 +2069,17 @@ development of Ruby gems.")
        (modify-phases %standard-phases
          (add-after 'unpack 'fix-test-include-path
           (lambda* (#:key inputs #:allow-other-keys)
-            (substitute* "Rakefile"
-              (("Hoe\\.add_include_dirs .*")
-               (string-append "Hoe.add_include_dirs \""
-                              (assoc-ref inputs "ruby-minitest-4")
-                              "/lib/ruby/gems/2.2.0/gems/minitest-"
-                              ,(package-version ruby-minitest-4)
-                              "/lib" "\"")))))
+             (let* ((minitest (assoc-ref inputs "ruby-minitest-4"))
+                    (minitest-home  (gem-home minitest
+                                              ,(package-version ruby))))
+               (substitute* "Rakefile"
+                 (("Hoe\\.add_include_dirs .*")
+                  (string-append "Hoe.add_include_dirs \""
+                                 minitest-home
+                                 "/gems/minitest-"
+                                 ,(package-version ruby-minitest-4)
+                                 "/lib" "\""))))
+             #t))
          (add-before 'check 'fix-test-assumptions
           (lambda _
             ;; The test output includes the file name, so a couple of tests
diff --git a/guix/build/ruby-build-system.scm b/guix/build/ruby-build-system.scm
index 2685da1..18da438 100644
--- a/guix/build/ruby-build-system.scm
+++ b/guix/build/ruby-build-system.scm
@@ -26,7 +26,8 @@
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:export (%standard-phases
-            ruby-build))
+            ruby-build
+            gem-home))
 
 ;; Commentary:
 ;;
@@ -136,3 +137,13 @@ GEM-FLAGS are passed to the 'gem' invokation, if present."
 (define* (ruby-build #:key inputs (phases %standard-phases)
                      #:allow-other-keys #:rest args)
   (apply gnu:gnu-build #:inputs inputs #:phases phases args))
+
+(define (gem-home store-path ruby-version)
+  "Return a string to the gem home directory in the store given a STORE-PATH
+and the RUBY-VERSION used to build that ruby package"
+  (string-append
+   store-path
+   "/lib/ruby/gems/"
+   (regexp-substitute #f
+                      (string-match "^[0-9]+\\.[0-9]+" ruby-version)
+                      0 ".0")))
-- 
2.6.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-gnu-ruby-Update-to-2.3.0.patch --]
[-- Type: text/x-patch; name="0004-gnu-ruby-Update-to-2.3.0.patch", Size: 4695 bytes --]

From 28dee96b0de99adf8657457660b9e3733434d2b4 Mon Sep 17 00:00:00 2001
From: Ben Woodcroft <donttrustben@gmail.com>
Date: Mon, 4 Jan 2016 09:38:42 +1000
Subject: [PATCH 4/4] gnu: ruby: Update to 2.3.0.

* gnu/packages/ruby.scm (ruby): Update to 2.3.0.
(ruby-2.2): New variable.
---
 gnu/packages/ruby.scm | 75 +++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 55 insertions(+), 20 deletions(-)

diff --git a/gnu/packages/ruby.scm b/gnu/packages/ruby.scm
index 71c4d05..c68e7d9 100644
--- a/gnu/packages/ruby.scm
+++ b/gnu/packages/ruby.scm
@@ -4,7 +4,7 @@
 ;;; Copyright © 2014, 2015 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2014, 2015 David Thompson <davet@gnu.org>
 ;;; Copyright © 2015 Ricardo Wurmus <rekado@elephly.net>
-;;; Copyright © 2015 Ben Woodcroft <donttrustben@gmail.com>
+;;; Copyright © 2015, 2016 Ben Woodcroft <donttrustben@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -46,7 +46,7 @@
 (define-public ruby
   (package
     (name "ruby")
-    (version "2.2.4")
+    (version "2.3.0")
     (source
      (origin
        (method url-fetch)
@@ -55,28 +55,30 @@
                            "/ruby-" version ".tar.xz"))
        (sha256
         (base32
-         "0g3ps4q3iz7wj9m45n8xyxzw8nh29ljdqb87b0f6i0p3853gz2yj"))))
+         "15s0dsb5ynf3d2w5gzawnszq5594fqvapv2y7a0qw16przq5l4kh"))))
     (build-system gnu-build-system)
     (arguments
      `(#:test-target "test"
-       #:parallel-tests? #f
        #:phases
-       (alist-cons-before
-        'configure 'replace-bin-sh
-        (lambda _
-          (substitute* '("Makefile.in"
-                         "ext/pty/pty.c"
-                         "io.c"
-                         "lib/mkmf.rb"
-                         "process.c"
-                         "test/rubygems/test_gem_ext_configure_builder.rb"
-                         "test/rdoc/test_rdoc_parser.rb"
-                         "test/ruby/test_rubyoptions.rb"
-                         "test/ruby/test_process.rb"
-                         "test/ruby/test_system.rb"
-                         "tool/rbinstall.rb")
-            (("/bin/sh") (which "sh"))))
-        %standard-phases)))
+       (modify-phases %standard-phases
+         (add-before 'configure 'replace-bin-sh
+           (lambda _
+             (substitute* '("Makefile.in"
+                            "ext/pty/pty.c"
+                            "io.c"
+                            "lib/mkmf.rb"
+                            "process.c"
+                            "test/rubygems/test_gem_ext_configure_builder.rb"
+                            "test/rdoc/test_rdoc_parser.rb"
+                            "test/ruby/test_rubyoptions.rb"
+                            "test/ruby/test_process.rb"
+                            "test/ruby/test_system.rb"
+                            "tool/rbinstall.rb")
+               (("/bin/sh") (which "sh")))
+             (substitute* "ext/fiddle/libffi-3.2.1/configure"
+               (("SHELL = /bin/sh")
+                (string-append "SHELL = " (which "sh"))))
+             #t)))))
     (inputs
      `(("readline" ,readline)
        ("openssl" ,openssl)
@@ -95,6 +97,39 @@ a focus on simplicity and productivity.")
     (home-page "https://ruby-lang.org")
     (license license:ruby)))
 
+(define-public ruby-2.2
+  (package (inherit ruby)
+    (version "2.2.4")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (string-append "http://cache.ruby-lang.org/pub/ruby/"
+                           (version-major+minor version)
+                           "/ruby-" version ".tar.bz2"))
+       (sha256
+        (base32
+         "0g3ps4q3iz7wj9m45n8xyxzw8nh29ljdqb87b0f6i0p3853gz2yj"))))
+    (arguments
+     `(#:test-target "test"
+       #:parallel-tests? #f
+       #:phases
+       (alist-cons-before
+        'configure 'replace-bin-sh
+        (lambda _
+          (substitute* '("Makefile.in"
+                         "ext/pty/pty.c"
+                         "io.c"
+                         "lib/mkmf.rb"
+                         "process.c"
+                         "test/rubygems/test_gem_ext_configure_builder.rb"
+                         "test/rdoc/test_rdoc_parser.rb"
+                         "test/ruby/test_rubyoptions.rb"
+                         "test/ruby/test_process.rb"
+                         "test/ruby/test_system.rb"
+                         "tool/rbinstall.rb")
+            (("/bin/sh") (which "sh"))))
+        %standard-phases)))))
+
 (define-public ruby-2.1
   (package (inherit ruby)
     (version "2.1.6")
-- 
2.6.3