[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: gem2deb: support for building using setup.rb


On 11/05/11 21:24, Antonio Terceiro wrote:
>>   OK. I've reverted the commits I had pushed to master, and stored them
>> on the setup-rb branch. I'll merge any change necessary from your commits.
> Ok, I've pushed my master branch there. It would be nice if you could
> provide a clean set of changes on top of that, with:
>   * one commit with the refactoring of the destdir calculations (that's
>     a nice catch, thanks!)
>   * one commit that adds the new class, the corresponding change in
>     the dh_ruby script for intantiating the correct class.

  ... I've split the later one, is it a huge deal ? I like
small-but-consistent commits (all intermediate commits run fine).

> About the code:
> Please add your new class to a new file with a proper name and add the
> proper require in dh_ruby.


> It would be nice to have automated tests for this new class.

  Hmmmm... A bit later ?

> The spawn method you wrote already exists: run(cmd). You can use it
> instead.


> Instead of adding a new debhelper buildsystem, would't it be better to
> centralize the logic of calling dh_ruby in one place? I'm not sure about
> this, because that code seems so simple already. :-/

  I prefer a separate buildsystem since I don't want the setuprb one to
trigger automatically (that would wreck havoc on the buildds, I
believe), and I don't see how this could be done simply inside the
dh_ruby thing.

  Could this be a variable from within debian/rules ? Is there an API
for reading those ? If not, that means adding quite a bit of code to
dh_ruby to parse debian/rules, I'm not sure we want that.

  For now, the attached patches keep with my first solution of a
separate build system.

> Your internal_install method doesn't handle running the tests. It would
> be nice to extract the logic of running tests into a method of its on,
> so that the install method would look like the following sequence:
>   * install files and build extensions
>   * run tests
>   * update shebangs
>   * check for require rubygems
> then you would only need to override the install files and build
> extensions part. Actually, check the updated master branch, it's easier
> to show than than to describe. :)

  Yes, that's perfect ! This way, the logic is clear and overriding is easy.

> http://git.debian.org/?p=pkg-ruby-extras/gem2deb.git;a=commitdiff;h=fb547d84b5545ac5dee8de9023ccae51d5cf61da
> So you should be good by overriding install_files_and_build_extensions
> on your new class.

  I've done that, and did as well for tests so far, but just stating it
is currently not supported. I need to dig a little more into this.

> after (or instead of) `setup.rb clean`, shouldn't you also do `setup.rb distclean`?
> (I think ruby-pkg-tools does something like that)

  I've changed to distclean.

  I've attached an archive of git format-patch, to avoid blundering too
much with the master repository.



Vincent Fourmond, Debian Developer

This thing was [...] incessantly making that flat honking noise of the
sort duck hunters make just before they are shot by other duck
 -- Terry Pratchet, Unseen Academicals

Vincent, listening to Mirrorage (Glasser)

Attachment: gem2deb-setuprb-patches.tar.gz
Description: application/gzip

Reply to: