[PATCH] gnu: Add ruby 1.8.7

[PATCH] gnu: Add ruby 1.8.7

[Pjotr Prins]

    * gnu/packages/ruby.scm: Added older Ruby 1.8.7
---
 gnu/packages/ruby.scm |   27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/gnu/packages/ruby.scm b/gnu/packages/ruby.scm
index ac751f5..31cbe54 100644
--- a/gnu/packages/ruby.scm
+++ b/gnu/packages/ruby.scm
@@ -89,6 +89,33 @@ a focus on simplicity and productivity.")
     (home-page "https://ruby-lang.org")
     (license license:ruby)))
 
+(define-public ruby-1.8
+  (package (inherit ruby)
+    (name "ruby")
+    (version "1.8.7-p374")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (string-append "ftp://ftp.ruby-lang.org/pub/ruby/1.8/"
+                           name "-" version ".tar.bz2"))
+     (sha256
+      (base32
+       "1qq7khilwkayrhwmzlxk83scrmiqfi7lgsn4c63znyvz2c1lgqxl"))))
+    (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")
+             (("/bin/sh") (which "sh"))))
+         %standard-phases)))))
+
 (define-public ruby-i18n
   (package
     (name "ruby-i18n")
-- 
1.7.10.4

Re: [PATCH] gnu: Add ruby 1.8.7

[Mark H Weaver]

Pjotr Prins <pjotr.public12@thebird.nl> writes:
>     * gnu/packages/ruby.scm: Added older Ruby 1.8.7

According to our conventions, this should be:

* gnu/packages/ruby.scm (ruby-1.8): New variable.

> ---
>  gnu/packages/ruby.scm |   27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/gnu/packages/ruby.scm b/gnu/packages/ruby.scm
> index ac751f5..31cbe54 100644
> --- a/gnu/packages/ruby.scm
> +++ b/gnu/packages/ruby.scm
> @@ -89,6 +89,33 @@ a focus on simplicity and productivity.")
>      (home-page "https://ruby-lang.org")
>      (license license:ruby)))
>  
> +(define-public ruby-1.8
> +  (package (inherit ruby)
> +    (name "ruby")

Is there a reason to repeat the 'name' field here?
It should be inherited.

> +    (version "1.8.7-p374")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append "ftp://ftp.ruby-lang.org/pub/ruby/1.8/"
> +                           name "-" version ".tar.bz2"))

In the ruby-2.1.3 package description, we used the http URI starting
with "http://cache.ruby-lang.org/pub/ruby/".  It's probably better to
use that one, since 'http' has some advantages and our http client is
more robust.

> +     (sha256
> +      (base32
> +       "1qq7khilwkayrhwmzlxk83scrmiqfi7lgsn4c63znyvz2c1lgqxl"))))
> +    (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")
> +             (("/bin/sh") (which "sh"))))
> +         %standard-phases)))))

It looks like the 'native-search-paths' field needs to be overridden,
since the existing one has "2.1.3" in it.

     Thanks!
       Mark

Re: [PATCH] gnu: Add ruby 1.8.7

[Pjotr Prins]

On Sun, Oct 26, 2014 at 07:33:51PM -0400, Mark H Weaver wrote:
> Pjotr Prins <pjotr.public12@thebird.nl> writes:
> >     * gnu/packages/ruby.scm: Added older Ruby 1.8.7
> 
> According to our conventions, this should be:
> 
> * gnu/packages/ruby.scm (ruby-1.8): New variable.

Is a new package a variable? 

We used the same message with ruby2.  Where can I find these
conventions, btw?

> > ---
> >  gnu/packages/ruby.scm |   27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/gnu/packages/ruby.scm b/gnu/packages/ruby.scm
> > index ac751f5..31cbe54 100644
> > --- a/gnu/packages/ruby.scm
> > +++ b/gnu/packages/ruby.scm
> > @@ -89,6 +89,33 @@ a focus on simplicity and productivity.")
> >      (home-page "https://ruby-lang.org")
> >      (license license:ruby)))
> >  
> > +(define-public ruby-1.8
> > +  (package (inherit ruby)
> > +    (name "ruby")
> 
> Is there a reason to repeat the 'name' field here?
> It should be inherited.

Good point. It is a remnant of trying things.

> > +    (version "1.8.7-p374")
> > +    (source
> > +     (origin
> > +       (method url-fetch)
> > +       (uri (string-append "ftp://ftp.ruby-lang.org/pub/ruby/1.8/"
> > +                           name "-" version ".tar.bz2"))
> 
> In the ruby-2.1.3 package description, we used the http URI starting
> with "http://cache.ruby-lang.org/pub/ruby/".  It's probably better to
> use that one, since 'http' has some advantages and our http client is
> more robust.

OK

> > +     (sha256
> > +      (base32
> > +       "1qq7khilwkayrhwmzlxk83scrmiqfi7lgsn4c63znyvz2c1lgqxl"))))
> > +    (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")
> > +             (("/bin/sh") (which "sh"))))
> > +         %standard-phases)))))
> 
> It looks like the 'native-search-paths' field needs to be overridden,
> since the existing one has "2.1.3" in it.

I don't think it is used by 1.8.7. Does that matter?

Pj.

Re: [PATCH] gnu: Add ruby 1.8.7

[Pjotr Prins]

On Mon, Oct 27, 2014 at 12:44:36AM +0100, Pjotr Prins wrote:
> > It looks like the 'native-search-paths' field needs to be overridden,
> > since the existing one has "2.1.3" in it.
> 
> I don't think it is used by 1.8.7. Does that matter?

How would I override it as just being empty?

Pj.

Re: [PATCH] gnu: Add ruby 1.8.7

[David Thompson]

Pjotr Prins <pjotr.public12@thebird.nl> writes:

> On Mon, Oct 27, 2014 at 12:44:36AM +0100, Pjotr Prins wrote:
>> > It looks like the 'native-search-paths' field needs to be overridden,
>> > since the existing one has "2.1.3" in it.
>> 
>> I don't think it is used by 1.8.7. Does that matter?
>
> How would I override it as just being empty?
>
> Pj.
>

(native-search-paths '())

Does Ruby 1.8.7 not know about GEM_PATH because rubygems was a separate
package at that time?

-- 
David Thompson
Web Developer - Free Software Foundation - http://fsf.org
GPG Key: 0FF1D807
Support the FSF: https://fsf.org/donate

Re: [PATCH] gnu: Add ruby 1.8.7

[Mark H Weaver]

Pjotr Prins <pjotr.public12@thebird.nl> writes:

> On Sun, Oct 26, 2014 at 07:33:51PM -0400, Mark H Weaver wrote:
>> Pjotr Prins <pjotr.public12@thebird.nl> writes:
>> >     * gnu/packages/ruby.scm: Added older Ruby 1.8.7
>> 
>> According to our conventions, this should be:
>> 
>> * gnu/packages/ruby.scm (ruby-1.8): New variable.
>
> Is a new package a variable? 

'ruby-1.8' is a Scheme variable that's bound to a package object.

I admit that I'm not fond of this convention -- I'd prefer "New package"
-- but it's what we've been doing, anyway.

> We used the same message with ruby2.  Where can I find these
> conventions, btw?

In broad strokes, we use the GNU changelog conventions:

  http://www.gnu.org/prep/standards/html_node/Change-Logs.html

but with some minor formatting differences.  However, the best thing is
to look in the commit history for examples close to what you're doing.

There is brief mention of this in the "Submitting Patches" section of
the HACKING file in the source tree.

>> It looks like the 'native-search-paths' field needs to be overridden,
>> since the existing one has "2.1.3" in it.
>
> I don't think it is used by 1.8.7. Does that matter?

Yes, otherwise users who install ruby 1.8.7 will be advised to set
GEM_PATH to $HOME/.guix-profile/lib/ruby/gems/2.1.3.

If setting GEM_PATH is not appropriate when using ruby 1.8.7, then you
should override it here to be the empty list, like so:

  (native-search-paths '())

     Thanks,
       Mark

Re: [PATCH] gnu: Add ruby 1.8.7

[Pjotr Prins]

On Sun, Oct 26, 2014 at 08:53:39PM -0400, David Thompson wrote:
> (native-search-paths '())

Ah, I tried #f.

> Does Ruby 1.8.7 not know about GEM_PATH because rubygems was a separate
> package at that time?

Yep. It was not a core feature at the time.

Pj.

Re: [PATCH] gnu: Add ruby 1.8.7

[Pjotr Prins]

On Sun, Oct 26, 2014 at 09:43:22PM -0400, Mark H Weaver wrote:
> In broad strokes, we use the GNU changelog conventions:
> 
>   http://www.gnu.org/prep/standards/html_node/Change-Logs.html
> 
> but with some minor formatting differences.  However, the best thing is
> to look in the commit history for examples close to what you're doing.
> 
> There is brief mention of this in the "Submitting Patches" section of
> the HACKING file in the source tree.

I read those earlier. I used a commit message we used before. I think
that if you are serious about convention you should point to a real
list of allowed standard messages. Now it looks arbitrary and
makes the submitter look stupid.

> >> It looks like the 'native-search-paths' field needs to be overridden,
> >> since the existing one has "2.1.3" in it.
> >
> > I don't think it is used by 1.8.7. Does that matter?
> 
> Yes, otherwise users who install ruby 1.8.7 will be advised to set
> GEM_PATH to $HOME/.guix-profile/lib/ruby/gems/2.1.3.

Ah, that is what it is for. Actually the build did not give me that
message. I DO get a load of messages such as

  strip:/gnu/store/icz59y35qqmm4jcrnkqx7d73xlzsq0lq-ruby-1.8.7-p374/lib/ruby/1.8/rexml/attlistdecl.rb:
  File format not recognized

is there a way to prevent files with a .rb extension or ruby shebang
to be stripped?

Pj.

Re: [PATCH] gnu: Add ruby 1.8.7

[Mark H Weaver]

Pjotr Prins <pjotr.public12@thebird.nl> writes:

> On Sun, Oct 26, 2014 at 09:43:22PM -0400, Mark H Weaver wrote:
>> In broad strokes, we use the GNU changelog conventions:
>> 
>>   http://www.gnu.org/prep/standards/html_node/Change-Logs.html
>> 
>> but with some minor formatting differences.  However, the best thing is
>> to look in the commit history for examples close to what you're doing.
>> 
>> There is brief mention of this in the "Submitting Patches" section of
>> the HACKING file in the source tree.
>
> I read those earlier. I used a commit message we used before. I think
> that if you are serious about convention you should point to a real
> list of allowed standard messages.

The commit log is full of examples, but I agree that it would be good to
have more documentation on this.  There's a lot to do, and we're a small
team.

> Now it looks arbitrary and makes the submitter look stupid.

I agree that the conventions are arbitrary, and could probably use
improvement, but I also think there's some value in keeping things
consistent.  I don't think it makes you look stupid.  I'm sorry if I
came off that way.

>> >> It looks like the 'native-search-paths' field needs to be overridden,
>> >> since the existing one has "2.1.3" in it.
>> >
>> > I don't think it is used by 1.8.7. Does that matter?
>> 
>> Yes, otherwise users who install ruby 1.8.7 will be advised to set
>> GEM_PATH to $HOME/.guix-profile/lib/ruby/gems/2.1.3.
>
> Ah, that is what it is for. Actually the build did not give me that
> message.

The message is supposed to be shown when you install the package using
"guix package -i", but I see that it wasn't working.  The reason is that
"lib/ruby/gems/2.1.3" doesn't actually exist in the built ruby package.
Instead, there's "lib/ruby/gems/2.1.0".  I guess that should be fixed in
a separate commit.

If the search-path-specification contains a directory that exists and is
not already in your GEM_PATH, you should see the message when you
install ruby, or when you run "guix package --search-paths".

> I DO get a load of messages such as
>
>   strip:/gnu/store/icz59y35qqmm4jcrnkqx7d73xlzsq0lq-ruby-1.8.7-p374/lib/ruby/1.8/rexml/attlistdecl.rb:
>   File format not recognized
>
> is there a way to prevent files with a .rb extension or ruby shebang
> to be stripped?

In theory the default 'strip' phase could be replaced by one that's more
selective, but please don't do that.

The problem happens in almost every package, so the improvements should
be made in the default 'strip' phase.  The relevant procedure is 'strip'
in guix/build/gnu-build-system.scm, but beware that changing that file
(or just about anything else in that directory) will trigger a full
rebuild of all packages.  It would be nice to do at some point, but it's
quite far down on our TODO list.

     Regards,
       Mark

Re: [PATCH] gnu: Add ruby 1.8.7

[Pjotr Prins]

On Sun, Oct 26, 2014 at 07:33:51PM -0400, Mark H Weaver wrote:
> > +(define-public ruby-1.8
> > +  (package (inherit ruby)
> > +    (name "ruby")
> 
> Is there a reason to repeat the 'name' field here?
> It should be inherited.

it won't compile without.

‘strip’ phase

[Ludovic Courtès]

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

>> I DO get a load of messages such as
>>
>>   strip:/gnu/store/icz59y35qqmm4jcrnkqx7d73xlzsq0lq-ruby-1.8.7-p374/lib/ruby/1.8/rexml/attlistdecl.rb:
>>   File format not recognized
>>
>> is there a way to prevent files with a .rb extension or ruby shebang
>> to be stripped?
>
> In theory the default 'strip' phase could be replaced by one that's more
> selective, but please don't do that.
>
> The problem happens in almost every package, so the improvements should
> be made in the default 'strip' phase.  The relevant procedure is 'strip'
> in guix/build/gnu-build-system.scm, but beware that changing that file
> (or just about anything else in that directory) will trigger a full
> rebuild of all packages.  It would be nice to do at some point, but it's
> quite far down on our TODO list.

I think the ‘strip’ phase is fine this way: sure, it ends up invoking
‘strip’ on files that are not ELF files, but it’s harmless.  So we get
these extra warnings in the build log, but they are really harmless.

Conversely, attempts to be more selective may miss actual ELF files.

WDYT?

Ludo’.

Re: ‘strip’ phase

[Mark H Weaver]

ludo@gnu.org (Ludovic Courtès) writes:

> Mark H Weaver <mhw@netris.org> skribis:
>
>>> I DO get a load of messages such as
>>>
>>>   strip:/gnu/store/icz59y35qqmm4jcrnkqx7d73xlzsq0lq-ruby-1.8.7-p374/lib/ruby/1.8/rexml/attlistdecl.rb:
>>>   File format not recognized
>>>
>>> is there a way to prevent files with a .rb extension or ruby shebang
>>> to be stripped?
>>
>> In theory the default 'strip' phase could be replaced by one that's more
>> selective, but please don't do that.
>>
>> The problem happens in almost every package, so the improvements should
>> be made in the default 'strip' phase.  The relevant procedure is 'strip'
>> in guix/build/gnu-build-system.scm, but beware that changing that file
>> (or just about anything else in that directory) will trigger a full
>> rebuild of all packages.  It would be nice to do at some point, but it's
>> quite far down on our TODO list.
>
> I think the ‘strip’ phase is fine this way: sure, it ends up invoking
> ‘strip’ on files that are not ELF files, but it’s harmless.  So we get
> these extra warnings in the build log, but they are really harmless.

Large numbers of spurious warnings are not harmless, because they make
it unlikely that anyone will notice the warnings worth our attention.
They condition us to ignore all warnings, because it's too much work to
look through them all.

> Conversely, attempts to be more selective may miss actual ELF files.

True, but this is more harmless than the spurious warnings, IMO.  It
merely means that we failed to save some disk space.

IMO, the best solution is to check for the presence of ELF headers
ourselves.  We already have ELF code in Guile master.

What do you think?

      Mark

Re: ‘strip’ phase

[Ludovic Courtès]

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

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

[...]

>> I think the ‘strip’ phase is fine this way: sure, it ends up invoking
>> ‘strip’ on files that are not ELF files, but it’s harmless.  So we get
>> these extra warnings in the build log, but they are really harmless.
>
> Large numbers of spurious warnings are not harmless, because they make
> it unlikely that anyone will notice the warnings worth our attention.
> They condition us to ignore all warnings, because it's too much work to
> look through them all.

Right.

>> Conversely, attempts to be more selective may miss actual ELF files.
>
> True, but this is more harmless than the spurious warnings, IMO.  It
> merely means that we failed to save some disk space.
>
> IMO, the best solution is to check for the presence of ELF headers
> ourselves.  We already have ELF code in Guile master.

Yes, that sounds good.  Instead of relying on Guile’s ELF code, perhaps
it would be enough to check for the presence of the ELF magic bytes?

Ludo’.

Re: ‘strip’ phase

[Mark H Weaver]

ludo@gnu.org (Ludovic Courtès) writes:

> Mark H Weaver <mhw@netris.org> skribis:
>
>> IMO, the best solution is to check for the presence of ELF headers
>> ourselves.  We already have ELF code in Guile master.
>
> Yes, that sounds good.  Instead of relying on Guile’s ELF code, perhaps
> it would be enough to check for the presence of the ELF magic bytes?

Agreed.  The test need not be perfect, because the worst that can happen
is an _occasional_ warning from 'strip', which really will be harmless :)

    Thanks,
      Mark

Re: ‘strip’ phase

[Ludovic Courtès]

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

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> Mark H Weaver <mhw@netris.org> skribis:
>>
>>> IMO, the best solution is to check for the presence of ELF headers
>>> ourselves.  We already have ELF code in Guile master.
>>
>> Yes, that sounds good.  Instead of relying on Guile’s ELF code, perhaps
>> it would be enough to check for the presence of the ELF magic bytes?
>
> Agreed.  The test need not be perfect, because the worst that can happen
> is an _occasional_ warning from 'strip', which really will be harmless :)

Commit 50b87bd in ‘core-updates’ does that.

Thanks,
Ludo’.