[PATCH] Gemspecs / Add ruby-ruby-engine.

[PATCH] Gemspecs / Add ruby-ruby-engine.

[Ben Woodcroft]

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

Hi Guix,

I've attached a patch for a simple rubygem. This one was slightly nasty 
because the gem for version 1.0.1 includes the .gem file for version 
1.0.0, which means that 1.0.0 gets silently installed instead of the 
built and tested 1.0.1 .gem file - it is unlucky that 
"pkg/ruby-engine-1.0.0.gem" is lexicographically before 
"ruby-engine-1.0.1.gem".

While I managed to install 1.0.1, I wasn't sure how best to remove the 
bundled 1.0.0 .gem file. The issue is that when the source is a .gem 
file (ie most of the time), the gemspec is taken from the downloaded 
.gem file directly, and in the same phase the .gem file is built. So as 
a packager there is no way to make changes to the gemspec without 
replacing the entire build phase. There's a number of rubygems that are 
contaminated with junk so it would be great for there to be a simple way 
to modify the gemspec before "gem build" is run.

Would someone with more experience like to suggest a way of doing this? 
A new "gemspec" phase before "build" that takes the gemspec out of the 
.gem so the packager can manipulate it perhaps?

It would also be good to check that there is only one .gem file.

Thanks,
ben

[-- Attachment #2: 0001-gnu-Add-ruby-ruby-engine.patch --]
[-- Type: text/x-patch, Size: 2424 bytes --]

From aeaf8255414669e5452647d800bf571b50699f29 Mon Sep 17 00:00:00 2001
From: Ben Woodcroft <donttrustben@gmail.com>
Date: Wed, 30 Dec 2015 16:51:18 +1000
Subject: [PATCH] gnu: Add ruby-ruby-engine.

* gnu/packages/ruby.scm (ruby-ruby-engine): New variable.
---
 gnu/packages/ruby.scm | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/gnu/packages/ruby.scm b/gnu/packages/ruby.scm
index a3eafb1..b81eef4 100644
--- a/gnu/packages/ruby.scm
+++ b/gnu/packages/ruby.scm
@@ -2884,3 +2884,46 @@ programs to concentrate on the implementation of network protocols.  It can be
 used to create both network servers and clients.")
     (home-page "http://rubyeventmachine.com")
     (license (list license:ruby license:gpl3)))) ; GPLv3 only AFAICT
+
+(define-public ruby-ruby-engine
+  (package
+    (name "ruby-ruby-engine")
+    (version "1.0.1")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (rubygems-uri "ruby_engine" version))
+       (sha256
+        (base32
+         "1d0sd4q50zkcqhr395wj1wpn2ql52r0fpwhzjfvi1bljml7k546v"))))
+    (build-system ruby-build-system)
+    (arguments
+     `(#:phases
+       (modify-phases %standard-phases
+         (add-before 'check 'clean-up
+           (lambda _
+             (delete-file "Gemfile.lock")
+             (substitute* "ruby_engine.gemspec"
+               ;; Remove unnecessary imports that would entail further
+               ;; dependencies.
+               ((".*<rdoc.*") "")
+               ((".*<rubygems-tasks.*") "")
+               ;; Remove extraneous .gem file
+               (("\\\"pkg/ruby_engine-1.0.0.gem\\\",") ""))
+             (substitute* "Rakefile"
+               (("require 'rubygems/tasks'") "")
+               (("Gem::Tasks.new") ""))
+             ;; Remove extraneous .gem file that otherwise gets installed.
+             (delete-file "pkg/ruby_engine-1.0.0.gem")
+             #t)))))
+    (native-inputs
+     `(("bundler" ,bundler)
+       ("ruby-rspec" ,ruby-rspec-2)))
+    (synopsis "Simplifies checking for Ruby implementation")
+    (description
+     "@code{ruby_engine} provides an RubyEngine class that can be used to check
+which implementation of Ruby is in use.  It can provide the interpreter name and
+provides query methods such as @{RubyEngine.mri?}.")
+    (home-page
+     "https://github.com/janlelis/ruby_engine")
+    (license license:expat)))
-- 
2.6.3

Re: [PATCH] Gemspecs / Add ruby-ruby-engine.

[Ricardo Wurmus]

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

> I've attached a patch for a simple rubygem. This one was slightly nasty 
> because the gem for version 1.0.1 includes the .gem file for version 
> 1.0.0, which means that 1.0.0 gets silently installed instead of the 
> built and tested 1.0.1 .gem file - it is unlucky that 
> "pkg/ruby-engine-1.0.0.gem" is lexicographically before 
> "ruby-engine-1.0.1.gem".

Ideally, this would be done in a snippet, but unfortunately
“ruby-build-system” does not support snippets yet.  “ruby-pygmentize”
also would need to use snippets to remove bundled or prebuilt stuff, but
for now it has to be done in a build phase.

> While I managed to install 1.0.1, I wasn't sure how best to remove the 
> bundled 1.0.0 .gem file. The issue is that when the source is a .gem 
> file (ie most of the time), the gemspec is taken from the downloaded 
> .gem file directly, and in the same phase the .gem file is built. So as 
> a packager there is no way to make changes to the gemspec without 
> replacing the entire build phase. There's a number of rubygems that are 
> contaminated with junk so it would be great for there to be a simple way 
> to modify the gemspec before "gem build" is run.

The “build” phase in the “ruby-build-system” is responsible for
rebuilding the gem from source.  The “unpack” phase unpacks the gem
archive.  This should allow you to modify the gemspec in a phase
injected between “unpack” and “build”, no?

Ideally, though, we’d add support for snippets, so that removing bundled
stuff doesn’t require a new build phase.

> Would someone with more experience like to suggest a way of doing this? 
> A new "gemspec" phase before "build" that takes the gemspec out of the 
> .gem so the packager can manipulate it perhaps?
>
> It would also be good to check that there is only one .gem file.

And do what when this check fails?  If included gems were removed in a
snippet they would never be seen at a later point, so I think the right
way to do this is support snippets.  Does this make sense?

Now on to the patch:

> +
> +(define-public ruby-ruby-engine
> +  (package
> +    (name "ruby-ruby-engine")
> +    (version "1.0.1")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (rubygems-uri "ruby_engine" version))
> +       (sha256
> +        (base32
> +         "1d0sd4q50zkcqhr395wj1wpn2ql52r0fpwhzjfvi1bljml7k546v"))))
> +    (build-system ruby-build-system)
> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         (add-before 'check 'clean-up

Is it possible to move this after “unpack” instead?  It’s just a
side-effect of the “check” phase that the gem is installed, so I think
it’s best to move this phase right after “unpack” (because we don’t need
any of this stuff for any of the phases until “check”).

Maybe you can also add a FIXME comment (as in “ruby-pygmentize”) stating
that this really should be done in a snippet.

> +           (lambda _
> +             (delete-file "Gemfile.lock")
> +             (substitute* "ruby_engine.gemspec"
> +               ;; Remove unnecessary imports that would entail further
> +               ;; dependencies.
> +               ((".*<rdoc.*") "")
> +               ((".*<rubygems-tasks.*") "")
> +               ;; Remove extraneous .gem file
> +               (("\\\"pkg/ruby_engine-1.0.0.gem\\\",") ""))
> +             (substitute* "Rakefile"
> +               (("require 'rubygems/tasks'") "")
> +               (("Gem::Tasks.new") ""))
> +             ;; Remove extraneous .gem file that otherwise gets installed.
> +             (delete-file "pkg/ruby_engine-1.0.0.gem")
> +             #t)))))
> +    (native-inputs
> +     `(("bundler" ,bundler)
> +       ("ruby-rspec" ,ruby-rspec-2)))
> +    (synopsis "Simplifies checking for Ruby implementation")
> +    (description
> +     "@code{ruby_engine} provides an RubyEngine class that can be used to check
> +which implementation of Ruby is in use.  It can provide the interpreter name and
> +provides query methods such as @{RubyEngine.mri?}.")

“ruby_engine” is a name, so I would not use @code here.  How about this:

  The ruby_engine package provides a @code{RubyEngine} class that can be
  used to check which implementation of Ruby is in use.  ...

> +    (home-page
> +     "https://github.com/janlelis/ruby_engine")

Please keep this on one line.
Otherwise it’s fine.  Thank you!

~~ Ricardo

Re: [PATCH] Gemspecs / Add ruby-ruby-engine.

[Ben Woodcroft]

On 05/01/16 21:36, Ricardo Wurmus wrote:
> Ben Woodcroft <b.woodcroft@uq.edu.au> writes:
[..]
>> While I managed to install 1.0.1, I wasn't sure how best to remove the
>> bundled 1.0.0 .gem file. The issue is that when the source is a .gem
>> file (ie most of the time), the gemspec is taken from the downloaded
>> .gem file directly, and in the same phase the .gem file is built. So as
>> a packager there is no way to make changes to the gemspec without
>> replacing the entire build phase. There's a number of rubygems that are
>> contaminated with junk so it would be great for there to be a simple way
>> to modify the gemspec before "gem build" is run.
> The “build” phase in the “ruby-build-system” is responsible for
> rebuilding the gem from source.  The “unpack” phase unpacks the gem
> archive.  This should allow you to modify the gemspec in a phase
> injected between “unpack” and “build”, no?
That's not what I'm getting from reading of the code, no. The build 
phase of the ruby build system unpacks the gemspec from the source .gem 
file and then immediately uses it to build the gem. So if a file is 
deleted in a snippet or otherwise then the "gem build" command fails 
because it cannot find the deleted file - the gemspec contains a list of 
files to include in the gem. Does that make sense? My suggestion is to 
add a gemspec phase before build so that packagers are given the 
opportunity to modify the gemspec without having to rewrite the entire 
build phase.
>> Would someone with more experience like to suggest a way of doing this?
>> A new "gemspec" phase before "build" that takes the gemspec out of the
>> .gem so the packager can manipulate it perhaps?
>>
>> It would also be good to check that there is only one .gem file.
> And do what when this check fails?  If included gems were removed in a
> snippet they would never be seen at a later point, so I think the right
> way to do this is support snippets.  Does this make sense?
It is fine to remove the offending files but the gemspec must be 
modified accordingly otherwise "gem build" fails.
>
> Now on to the patch:
>
>> +
>> +(define-public ruby-ruby-engine
>> +  (package
>> +    (name "ruby-ruby-engine")
>> +    (version "1.0.1")
>> +    (source
>> +     (origin
>> +       (method url-fetch)
>> +       (uri (rubygems-uri "ruby_engine" version))
>> +       (sha256
>> +        (base32
>> +         "1d0sd4q50zkcqhr395wj1wpn2ql52r0fpwhzjfvi1bljml7k546v"))))
>> +    (build-system ruby-build-system)
>> +    (arguments
>> +     `(#:phases
>> +       (modify-phases %standard-phases
>> +         (add-before 'check 'clean-up
> Is it possible to move this after “unpack” instead?  It’s just a
> side-effect of the “check” phase that the gem is installed, so I think
> it’s best to move this phase right after “unpack” (because we don’t need
> any of this stuff for any of the phases until “check”).
>
> Maybe you can also add a FIXME comment (as in “ruby-pygmentize”) stating
> that this really should be done in a snippet.
Unfortunately we cannot move it since the build phase will then fail for 
the above reason.
>
>> +           (lambda _
>> +             (delete-file "Gemfile.lock")
>> +             (substitute* "ruby_engine.gemspec"
>> +               ;; Remove unnecessary imports that would entail further
>> +               ;; dependencies.
>> +               ((".*<rdoc.*") "")
>> +               ((".*<rubygems-tasks.*") "")
>> +               ;; Remove extraneous .gem file
>> +               (("\\\"pkg/ruby_engine-1.0.0.gem\\\",") ""))
>> +             (substitute* "Rakefile"
>> +               (("require 'rubygems/tasks'") "")
>> +               (("Gem::Tasks.new") ""))
>> +             ;; Remove extraneous .gem file that otherwise gets installed.
>> +             (delete-file "pkg/ruby_engine-1.0.0.gem")
>> +             #t)))))
>> +    (native-inputs
>> +     `(("bundler" ,bundler)
>> +       ("ruby-rspec" ,ruby-rspec-2)))
>> +    (synopsis "Simplifies checking for Ruby implementation")
>> +    (description
>> +     "@code{ruby_engine} provides an RubyEngine class that can be used to check
>> +which implementation of Ruby is in use.  It can provide the interpreter name and
>> +provides query methods such as @{RubyEngine.mri?}.")
> “ruby_engine” is a name, so I would not use @code here.  How about this:
>
>    The ruby_engine package provides a @code{RubyEngine} class that can be
>    used to check which implementation of Ruby is in use.  ...
ok
>
>> +    (home-page
>> +     "https://github.com/janlelis/ruby_engine")
> Please keep this on one line.
> Otherwise it’s fine.  Thank you!
No problem. I'm happy to send a follow up patch if you like, but would 
prefer to resolve the larger problem first.

Thanks for the review.
ben

Re: [PATCH] Gemspecs / Add ruby-ruby-engine.

[Ricardo Wurmus]

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

> On 05/01/16 21:36, Ricardo Wurmus wrote:
>> Ben Woodcroft <b.woodcroft@uq.edu.au> writes:
> [..]
>>> While I managed to install 1.0.1, I wasn't sure how best to remove the
>>> bundled 1.0.0 .gem file. The issue is that when the source is a .gem
>>> file (ie most of the time), the gemspec is taken from the downloaded
>>> .gem file directly, and in the same phase the .gem file is built. So as
>>> a packager there is no way to make changes to the gemspec without
>>> replacing the entire build phase. There's a number of rubygems that are
>>> contaminated with junk so it would be great for there to be a simple way
>>> to modify the gemspec before "gem build" is run.
>> The “build” phase in the “ruby-build-system” is responsible for
>> rebuilding the gem from source.  The “unpack” phase unpacks the gem
>> archive.  This should allow you to modify the gemspec in a phase
>> injected between “unpack” and “build”, no?
>
> That's not what I'm getting from reading of the code, no. The build 
> phase of the ruby build system unpacks the gemspec from the source .gem 
> file and then immediately uses it to build the gem.

Oh, you are right!  The gemspec file is deleted if it exists and then
extracted from the source gem.  Sorry, I should have looked at the code
more carefully.

I think extracting the gemspec should be done during the “unpack” phase,
or as you suggest during an additional “gemspec” phase.

But how does this relate to snippets?  In other build systems snippets
cause the source archive to be unpacked, modified, and then repacked.
If we split unpacking and gemspec extraction we’d have to make sure that
this also works when snippets are involved, i.e. snippets should see the
extracted gemspec (as the original one is discarded by the build system
anyway).

> It is fine to remove the offending files but the gemspec must be 
> modified accordingly otherwise "gem build" fails.

You are right.

>>> +    (arguments
>>> +     `(#:phases
>>> +       (modify-phases %standard-phases
>>> +         (add-before 'check 'clean-up
>> Is it possible to move this after “unpack” instead?  It’s just a
>> side-effect of the “check” phase that the gem is installed, so I think
>> it’s best to move this phase right after “unpack” (because we don’t need
>> any of this stuff for any of the phases until “check”).
>>
>> Maybe you can also add a FIXME comment (as in “ruby-pygmentize”) stating
>> that this really should be done in a snippet.
> Unfortunately we cannot move it since the build phase will then fail for 
> the above reason.

True.

>> Please keep this on one line.
>> Otherwise it’s fine.  Thank you!
> No problem. I'm happy to send a follow up patch if you like, but would 
> prefer to resolve the larger problem first.

Agreed.  I’d be happy if someone else could chime in to give an opinion
on whether to add a new phase or merge gemspec extraction with “unpack”.

~~ Ricardo

Re: [PATCH] Gemspecs / Add ruby-ruby-engine.

[Ludovic Courtès]

Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> skribis:

> But how does this relate to snippets?  In other build systems snippets
> cause the source archive to be unpacked, modified, and then repacked.
> If we split unpacking and gemspec extraction we’d have to make sure that
> this also works when snippets are involved, i.e. snippets should see the
> extracted gemspec (as the original one is discarded by the build system
> anyway).

The implementation of snippets is in ‘patch-and-repack’ procedure; it is
completely independent of build systems and their phases.

> Agreed.  I’d be happy if someone else could chime in to give an opinion
> on whether to add a new phase or merge gemspec extraction with “unpack”.

I think I don’t understand the issue at hand sufficiently well.  Two
things come to mind: “extract” and “unpack” sound synonymous; OTOH,
having a separate phase provides more flexibility, since one can remove
it, replace it, etc.

Ludo’.