Dne 2.7.2013 18:22, Troy Dawson napsal(a):
Hi,
Since this is my review request, I thank you both for your help with
this package. I will reply inline.
On 07/02/2013 08:42 AM, Vít Ondruch wrote:
> Dne 2.7.2013 14:48, Axilleas Pi napsal(a):
>> Hi all,
>>
>> I am in the process of an informal review attempt of rugged [0] and I
>> bumped into some errors that rpmlint found and could be false
>> positives. I would like your opinion.
>>
>>
>> 1) explicit-lib-dependency libgit2
>> You must let rpm find the library dependencies by itself. Do not put
>> unneeded explicit Requires: tags.
>>
>> I have found only this section in the wiki[1] which is a little
>> relevant, but some example would be better.
>
> If package has BR on some -devel package and the library is linked
> against it, then RPM automatically pick-ups the dependency on that
> library. You don't have to specify it manually.
>
> In this case, I am afraid that libgit2 is vendored in the gem and it is
> statically linked. That means:
>
> 1) It is against bundling policy
> 2) RPM cannot pickup the dependency
> 3) It makes ironically the Requires unneeded.
>
> I would focus on solving the first point ;)
>
Good catch. I hadn't noticed that there was anything bundled there.
I believe I've asked/commented on this before. Removing bundled
libraries is a pain with the current %prep setup.
This is due to the gemspec being created from the original gem. So you
have to remove the vendor directory, and then do some form of
sed/perl/whatever to get all the vendor files out of the gemspec you
just created.
You don't have to remove the vendor directory in this case, you just
need to properly setup the build environment if I read the extconf.rb
properly.
>>
>> 3) arch-dependent-file-in-usr-share
>> /usr/share/gems/gems/rugged-0.16.0/lib/rugged/rugged.so
>> This package installs an ELF binary in the /usr/share hierarchy,
>> which is reserved for architecture-independent files.
>
> See 2) This file should be moved into %{gem_extdir_mri}
>
This file should be removed from lib/rugged/, it's not supposed to be
there.
We move the ext/rugged/rugged.so over to %{gem_extdir_mri}, as is
standard procedure.
I hadn't noticed that the build copies the .so file over to
lib/rugged/rugged.so, where it shouldn't be.
Wrong ... There is attempt to explain how this works in Ruby Guidelines,
but it might not be clear enough. RubyGems always install the .so file
into the first search patch specified in .gemspec, which is typically
lib/ directory. Therefore, the correct installation procedure is to move
the .so file from lib/ into %{gem_extdir_mri} and the whole ext/
directory should be ignored/removed/not touched at all.
Vít