[Bug 540996] Review Request: rubygem-ffi - Foreign Function Interface package for Ruby

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=540996

Bryan Kearney <bkearney at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|needinfo?(bkearney at redhat.c |
                   |om)                         |

--- Comment #6 from Bryan Kearney <bkearney at redhat.com> 2010-02-15 11:28:43 EST ---
Sorry.. $DAYJOB took over. I have updated the spec file. Latest files:

SRPM: http://bkearney.fedorapeople.org/rubygem-ffi-0.5.4-1.fc12.src.rpm
SPEC File: http://bkearney.fedorapeople.org/rubygem-ffi.spec

rpmlint output:
[bkearney at localhost i686]$ rpmlint *
rubygem-ffi.i686: W: hidden-file-or-dir
/usr/lib/ruby/gems/1.8/gems/ffi-0.5.4/.require_paths
2 packages and 0 specfiles checked; 0 errors, 1 warnings.


koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1988494

Specific comments:


(In reply to comment #4)
> Some notes:
> 
> * %define -> %global
>   - We now prefer to use %global %instead of %define
>    
> https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define

Done.

> 
> 
> * Requires
>   - Please use "Requires: rubygem(rake-compiler)" style, ref:
>     https://fedoraproject.org/wiki/Packaging/Perl#Perl_Requires_and_Provides
> 
> * strip / debuginfo rpm
>   - Don't use ELF binary by yourself and create debuginfo rpm correctly.
>     ! Note
>       To create debuginfo rpm correctly, you have to compile C source
>       under %_builddir
>       ( i.e. when rubygem contains C extention library, you cannot install
>         gem file into %buildroot directly. You have to once install gem file
>         under directory created by %setup and then copy the whole tree
>         to %buildroot for correct debuginfo rpm creation:
>        
> https://fedoraproject.org/wiki/Packaging/Ruby#Ruby_Gem_with_extension_libraries_written_in_C
> )

I will be honest, I do not understand the value here. But I believe the new
spec file to be in line with these requirements.

> 
>     ! Note 2
>       Also, build log shows that compilation of C source to create
>       libffi_c.so is executed both under %_builddir and %buildroot, this
>       is just redundant.

Should only be done in %_builddir

> 
> * macro usage
>   - %ruby_sitelib macro seems to be used nowhere and unneeded.

removed.

> 
>   - Please use the defined %geminstdir also in %files

done.

> 
>   - You should not use %buildroot vs $RPM_BUILD_ROOT, %optflags vs
> $RPM_OPT_FLAGS
>     with mixed style and should choose one style.
>    
> https://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS

done

> 
> * Documents location
> --------------------------------------------------------------
> %doc README.rdoc LICENSE History.txt
> --------------------------------------------------------------
>   - These files should be installed under %geminstdir (in fact
>     2 of them are already installed).
>     Also document files under %geminstdir should be marked
>     as %doc properly.    
updated.

Thank you for the review.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.