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

[Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de>]

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