[Bug 622946] Review Request: rubygem-bcrypt-ruby - Wrapper Around bcrypt() password hashing algorithm

bugzilla at redhat.com bugzilla at redhat.com
Thu Aug 12 19:46:52 UTC 2010


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=622946

--- Comment #1 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp> 2010-08-12 15:46:51 EDT ---
Some notes:

* %define -> %global
  - Now we prefer to use %global instead of %define:
   
https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define

* License
  - Almost all files are under 4 clause BSD, except that
./ext/jruby/bcrypt_jruby/BCrypt.java
    is under MIT.
    The license tag should be "BSD with advertising and MIT".

* BuildRoot
  - BuildRoot tag is no longer needed on Fedora and EPEL6:
    https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

* Make build log more verbose
  - Please add "-V" option to "gem install" so that we can check if Fedora
specific
    compilation flags are correctly honored from build.log.

* Arch dependent files / C source codes / etc handling
  - .c / .h source codes, Makefile, extconf.rb are all needed for creating .so
file and
    should not be needed on runtime.
      - i.e. %geminstdir/ext/mri should be removed.
  - .o files are needed for creating .so file and should not be installed in
binary
    rpm (i.e. please remove %ruby_sitearch/%gemname/*.o ) (also see below)
  - %{geminstdir}/lib/*.so symlink should not be needed (putting "real" .so
files
    under %ruby_sitearch should be enough) (also see below)
  - %{geminstdir}/lib/bcrypt.rb is really needed and not %doc.
  - bcrypt_ext.so must be under %ruby_sitearch, not under
%ruby_sitearch/%gemname
    (lib/bcrypt.so reads:
-------------------------------------------------------------------
     6  else
     7    $LOAD_PATH.unshift(File.expand_path(File.join(File.dirname(__FILE__),
"..", "ext", "mri")))
     8    require "bcrypt_ext"
     9    require "openssl"
    10  end
-------------------------------------------------------------------
    So bcrypt_ext.so must be under ruby default search path (i.e ruby -e 'puts
$:').

-- 
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.



More information about the package-review mailing list