Bug 621015 - Review Request: rubygem-authlogic - A simple ruby authentication solution
Summary: Review Request: rubygem-authlogic - A simple ruby authentication solution
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 622946
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-08-04 01:12 UTC by Mo Morsi
Modified: 2010-10-28 05:46 UTC (History)
3 users (show)

Fixed In Version: rubygem-authlogic-2.1.6-4.fc14
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-10-20 17:07:19 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
tcallawa: fedora-cvs+


Attachments (Terms of Use)

Description Mo Morsi 2010-08-04 01:12:13 UTC
Spec URL: http://mo.morsi.org/files/gems/rubygem-authlogic.spec
SRPM URL: http://mo.morsi.org/files/gems/rubygem-authlogic-2.1.5-1.fc13.src.rpm

Description: 
A clean, simple, and unobtrusive ruby authentication solution.

$ rpmlint rpmbuild/SRPMS/rubygem-authlogic-2.1.5-1.fc13.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

$ rpmlint rpmbuild/RPMS/noarch/rubygem-authlogic-2.1.5-1.fc13.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.


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

Comment 1 Mamoru TASAKA 2010-08-05 17:52:28 UTC
Some notes:

* Unused macro
  - %ruby_sitelib seems to be used nowhere.

* Latest version
  - Latest version seems 2.1.6.

* Requires
  - ">= 0" part on "R: rubygem(activesupport)" is redundant.
  - "R: ruby(abi) = 1.8" is mandatory

* Documents
  - "%{geminstdir}/test" should be marked as %doc
  - Also "%{geminstdir}/Rakefile" can be marked as %doc (this is
    like "Makefile" in autotools system).

* %check
  - rubygem(jeweler) is not strictly needed for "$ rake test",
    so please enable "$ rake test".

  ! Some notes:
    - Without rubygem(ruby-debug) being installed, "$ rake test"
      aborts like
----------------------------------------------------------------
$ rake test --trace
(in /home/tasaka1/rpmbuild/INSTROOT/rubygem-authlogic-2.1.5-1.fc-foo-tasaka1/usr/lib/ruby/gems/1.8/gems/authlogic-2.1.5)
Jeweler (or a dependency) not available. Install it with: sudo gem install jeweler
** Invoke test (first_time)
** Execute test
....
....
/usr/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:31:in `gem_original_require': no such file to load -- ruby-debug (LoadError)
	from /usr/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:31:in `require'
	from ./test/test_helper.rb:3
	from ./test/i18n_test.rb:1:in `require'
	from ./test/i18n_test.rb:1
	from /usr/lib/ruby/gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:4:in `load'
	from /usr/lib/ruby/gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:4
	from /usr/lib/ruby/gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:4:in `each'
	from /usr/lib/ruby/gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:4
rake aborted!
----------------------------------------------------------------
      However, in fact "ruby-debug" does not seem to be needed.

    - After commenting out 'require "ruby-debug"' from ./test/test_helper.rb,
      some tests fails like
----------------------------------------------------------------
$ rake test --trace
(in /home/tasaka1/rpmbuild/INSTROOT/rubygem-authlogic-2.1.5-1.fc-foo-tasaka1/usr/lib/ruby/gems/1.8/gems/authlogic-2.1.5)
Jeweler (or a dependency) not available. Install it with: sudo gem install jeweler
** Invoke test (first_time)
** Execute test
....
....
  1) Error:
test_transitioning_password(ActsAsAuthenticTest::PasswordTest):
NameError: uninitialized constant BCrypt
    /usr/lib/ruby/gems/1.8/gems/activesupport-2.3.5/lib/active_support/dependencies.rb:443:in `load_missing_constant'
    /usr/lib/ruby/gems/1.8/gems/activesupport-2.3.5/lib/active_support/dependencies.rb:80:in `const_missing'
    /usr/lib/ruby/gems/1.8/gems/activesupport-2.3.5/lib/active_support/dependencies.rb:92:in `const_missing'
    ./test/../lib/authlogic/crypto_providers/bcrypt.rb:83:in `new_from_hash'
    ./test/../lib/authlogic/crypto_providers/bcrypt.rb:60:in `matches?'
    ./test/../lib/authlogic/acts_as_authentic/password.rb:259:in `valid_password?'
    /usr/lib/ruby/gems/1.8/gems/activesupport-2.3.5/lib/active_support/callbacks.rb:207:in `each_with_index'
    ./test/../lib/authlogic/acts_as_authentic/password.rb:253:in `each'
    ./test/../lib/authlogic/acts_as_authentic/password.rb:253:in `each_with_index'
    ./test/../lib/authlogic/acts_as_authentic/password.rb:253:in `valid_password?'
    ./test/acts_as_authentic_test/password_test.rb:224:in `transition_password_to'
    ./test/acts_as_authentic_test/password_test.rb:221:in `each'
    ./test/acts_as_authentic_test/password_test.rb:221:in `transition_password_to'
    ./test/acts_as_authentic_test/password_test.rb:163:in `test_transitioning_password'
    /usr/lib/ruby/gems/1.8/gems/activesupport-2.3.5/lib/active_support/testing/setup_and_teardown.rb:62:in `__send__'
    /usr/lib/ruby/gems/1.8/gems/activesupport-2.3.5/lib/active_support/testing/setup_and_teardown.rb:62:in `run'
---------------------------------------------------------------
      Maybe it is better that you package bcrypt-ruby?
      http://bcrypt-ruby.rubyforge.org/

Comment 2 Mo Morsi 2010-08-10 21:04:09 UTC
(In reply to comment #1)
> Some notes:
> 
> * Unused macro
>   - %ruby_sitelib seems to be used nowhere.

Removed

> 
> * Latest version
>   - Latest version seems 2.1.6.

Updated

> 
> * Requires
>   - ">= 0" part on "R: rubygem(activesupport)" is redundant.
>   - "R: ruby(abi) = 1.8" is mandatory

Done

> 
> * Documents
>   - "%{geminstdir}/test" should be marked as %doc
>   - Also "%{geminstdir}/Rakefile" can be marked as %doc (this is
>     like "Makefile" in autotools system).

Done

> 
> * %check
>   - rubygem(jeweler) is not strictly needed for "$ rake test",
>     so please enable "$ rake test".

Done

> 
>   ! Some notes:
>     - Without rubygem(ruby-debug) being installed, "$ rake test"
>       aborts like
> ----------------------------------------------------------------
> $ rake test --trace
> <snip>
> ----------------------------------------------------------------
>       However, in fact "ruby-debug" does not seem to be needed.
> 
>     - After commenting out 'require "ruby-debug"' from ./test/test_helper.rb,
>       some tests fails like

Done, patch0 added to do this.

> ----------------------------------------------------------------
> $ rake test --trace
> <snip>
> ---------------------------------------------------------------
>       Maybe it is better that you package bcrypt-ruby?
>       http://bcrypt-ruby.rubyforge.org/    

Package review request submitted 
http://bugzilla.redhat.com/show_bug.cgi?id=622946

rubygem(bcrypt-ruby) is added as a BR(check) dependency. Once added, rake test successfully completes.

Updated SRPM / Spec here:
pec URL: http://mo.morsi.org/files/gems/rubygem-authlogic.spec
SRPM URL: http://mo.morsi.org/files/gems/rubygem-authlogic-2.1.6-1.fc13.src.rpm

Comment 3 Mamoru TASAKA 2010-09-04 16:18:03 UTC
For 2.1.6-1

* BR
  - So would you at least add "BR(check): rubygem(bcrypt-ruby)" (and
    "BR: rubygem(rake)" so as to make %check section pass correctly?

Some additional notes
* BuildRoot
  - BuildRoot tag is no longer used (and needed) on Fedora and EPEL6
    https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

* ri files generation
  - As I said in other review request (I suppose), usually we
    generate ri documentation files, which can be used with
    ri (in ruby-ri) command.

* -doc files generation
  - And would you consider to create -doc subpackages and move
    document files (except for LICENSE, README.rdoc, CHANGELOG.rdoc)
    to -doc subpackage?

Comment 4 Mamoru TASAKA 2010-09-22 17:14:06 UTC
What is the status of this bug?

Comment 5 Mamoru TASAKA 2010-10-03 19:33:01 UTC
ping again?

Comment 6 Mo Morsi 2010-10-12 17:46:35 UTC
Apologies for the delay

(In reply to comment #3)
> For 2.1.6-1
> 
> * BR
>   - So would you at least add "BR(check): rubygem(bcrypt-ruby)" (and
>     "BR: rubygem(rake)" so as to make %check section pass correctly?
> 
> Some additional notes
> * BuildRoot
>   - BuildRoot tag is no longer used (and needed) on Fedora and EPEL6
>     https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
> 
> * ri files generation
>   - As I said in other review request (I suppose), usually we
>     generate ri documentation files, which can be used with
>     ri (in ruby-ri) command.
> 
> * -doc files generation
>   - And would you consider to create -doc subpackages and move
>     document files (except for LICENSE, README.rdoc, CHANGELOG.rdoc)
>     to -doc subpackage?

Everything has been taken care of. Also had to add patch1 to remove the jeweler dependency which is not needed.

New uploads:
Spec URL: http://mo.morsi.org/files/gems/rubygem-authlogic.spec
SRPM URL: http://mo.morsi.org/files/gems/rubygem-authlogic-2.1.6-2.fc13.src.rpm

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

Comment 7 Mamoru TASAKA 2010-10-12 19:57:24 UTC
Two comments

- I usually think that writing %doc attribute on -doc subpackage is
  redundant because the name of the rpm already shows that the rpm
  is for documentation.

- Maybe "Patch0: remove-unneeded-rubydebug-dep.patch" is no longer
  needed, because now rubygem-ruby-debug is available on F-12/13/14/15.
  (please try to remove Patch0 and adding BR: rubygem(ruby-debug))

Not blockers, approving.
---------------------------------------------------------
    This package (rubygem-authlogic) is APPROVED
    by mtasaka
---------------------------------------------------------

Comment 8 Mo Morsi 2010-10-12 20:46:01 UTC
Ok, thanks for the approval, just quickly took care of those last two things you mentioned. For reference here are the newest links:

Spec URL: http://mo.morsi.org/files/gems/rubygem-authlogic.spec
SRPM URL: http://mo.morsi.org/files/gems/rubygem-authlogic-2.1.6-3.fc13.src.rpm
Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2530947

New Package SCM Request
=======================
Package Name: rubygem-authlogic
Short Description: A simple ruby authentication solution
Owners: mmorsi
Branches: f13 f14
InitialCC:

Comment 9 Tom "spot" Callaway 2010-10-13 15:12:18 UTC
Git done (by process-git-requests).

Comment 10 Mamoru TASAKA 2010-10-13 18:45:57 UTC
Sorry, for directory ownership issue, -doc subpackage
should have "Requires: %{name} = %{version}-%{release}".

Comment 11 Fedora Update System 2010-10-13 20:23:51 UTC
rubygem-authlogic-2.1.6-4.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/rubygem-authlogic-2.1.6-4.fc14

Comment 12 Fedora Update System 2010-10-13 20:24:58 UTC
rubygem-authlogic-2.1.6-4.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/rubygem-authlogic-2.1.6-4.fc13

Comment 13 Mo Morsi 2010-10-13 20:25:51 UTC
Ah good catch, though I had already pushed the rpm to git before I noticed this.

Pushed a followup, adding the requires, and submitted the package to bodhi.

Comment 14 Fedora Update System 2010-10-14 06:29:00 UTC
rubygem-authlogic-2.1.6-4.fc13 has been pushed to the Fedora 13 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update rubygem-authlogic'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/rubygem-authlogic-2.1.6-4.fc13

Comment 15 Mamoru TASAKA 2010-10-20 17:07:19 UTC
Closing.

Comment 16 Fedora Update System 2010-10-27 22:42:41 UTC
rubygem-authlogic-2.1.6-4.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 17 Fedora Update System 2010-10-28 05:46:29 UTC
rubygem-authlogic-2.1.6-4.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.