Bug 1015778 - Review Request: rubygem-ruby-openid - A library for consuming and serving OpenID identities
Summary: Review Request: rubygem-ruby-openid - A library for consuming and serving Ope...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 903854
TreeView+ depends on / blocked
 
Reported: 2013-10-05 10:53 UTC by Ken Dreyer
Modified: 2013-11-10 06:09 UTC (History)
2 users (show)

Fixed In Version: rubygem-ruby-openid-2.3.0-3.fc20
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-11-08 04:31:00 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
license analysis result (432 bytes, text/plain)
2013-10-17 07:16 UTC, Mamoru TASAKA
no flags Details

Description Ken Dreyer 2013-10-05 10:53:03 UTC
Spec URL: http://ktdreyer.fedorapeople.org/reviews/rubygem-ruby-openid.spec
SRPM URL: http://ktdreyer.fedorapeople.org/reviews/rubygem-ruby-openid-2.3.0-1.fc21.src.rpm
Description: The Ruby OpenID library, with batteries included.

A Ruby library for verifying and serving OpenID identities.
Ruby OpenID makes it easy to add OpenID authentication to
your web applications.

Fedora Account System Username: ktdreyer

This is a rename of the "ruby-openid" package. As such, it follows http://fedoraproject.org/wiki/Package_Renaming_Process

Comment 1 Ken Dreyer 2013-10-05 10:54:12 UTC
rpmlint output:

$ rpmlint rubygem-ruby-openid-2.3.0-1.fc21.src.rpm rubygem-ruby-openid.spec 
1 packages and 1 specfiles checked; 0 errors, 0 warnings.

F21 scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6027258

Comment 2 Mamoru TASAKA 2013-10-15 09:50:42 UTC
I will take this.

Meanwhile, while my review request bug 912961 was already assigned to other people, would you take and review it?

Comment 3 Mamoru TASAKA 2013-10-17 07:15:19 UTC
Initial notes:

* License
  - It seems that the correct license tag for -doc subpackage
    should be "ASL 2.0 and LGPLv2+ and MIT". Would you
    check this? (see attached)
    Also, it is preferred that some explanation is written
    on spec file about some detailed license information
    (or including license information notes in source rpm)

  * Note that the licenses of files under test/data is somewhat
    unclear. Looking at linkparse.txt first and next the rest
    files, it seems that these files are copied from 
    python openid (see:
    https://svn.apache.org/repos/asf/incubator/heraldry/libraries/python/openid/trunk/openid/test/linkparse.txt
    https://svn.apache.org/repos/asf/incubator/heraldry/libraries/python/openid/trunk/COPYING
    ). For now I don't think this is a blocker, however please
    try to clarify.

* Improper Obsoletes
  - Obsoletes: ruby-openid  = 2.1.7-11 obsoletes "ruby-openid  = 2.1.7-11"
    _only_ (not "no more than").

* Filtering depedendency from examples/ directory
  - The common way for this is to remove executable permission bits from
    all files under examples/ directory.

* Notes for documents
  - Files like "INSTALL.md" is in most cases not needed, because we install
    the software using packaged rpm (i.e. not by following the method written
    in INSTALL.md)

  - I recommend to move "README.md" to main package, because it says "README",
    indicating the upstream want users to read this.

Comment 4 Mamoru TASAKA 2013-10-17 07:16:09 UTC
Created attachment 813204 [details]
license analysis result

My license analysis result

Comment 5 Mamoru TASAKA 2013-10-17 12:03:20 UTC
Additional notes:

* Test suite files
  - Current ruby guidelines says not to include files under
    test/ into binary rpm:
    https://fedoraproject.org/wiki/Packaging:Ruby?rd=Packaging/Ruby#Running_test_suites
    See "Do not ship tests" . If you exclude test/ directory from
    -doc subpackage, "LGPLv2+" license tag is not needed.

Comment 6 Ken Dreyer 2013-10-24 08:47:56 UTC
Thanks, I've filed https://github.com/openid/ruby-openid/issues/60 about the license question. I'm also excluding the test directory as you suggest, so we hopefully avoid the issue.

* Thu Oct 24 2013 Ken Dreyer <ktdreyer> - 2.3.0-2
- Updates for review request (RHBZ #1015778)
- Update license
- Clean up whitespace
- Adjust permissions on "examples" directory
- Add link to upstream test suite encoding bug
- Move README.md to main package
- Exclude INSTALL.md file and "test" directory

Specific changes (in git): http://fedorapeople.org/cgit/ktdreyer/public_git/rubygem-ruby-openid.git/commit/?id=75b66ed0e97b2a102899cd15b6bcbebf19cf0922

Spec: http://ktdreyer.fedorapeople.org/reviews/rubygem-ruby-openid.spec
SRPM: http://ktdreyer.fedorapeople.org/reviews/rubygem-ruby-openid-2.3.0-2.fc21.src.rpm

F21 scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6092974

Comment 7 Mamoru TASAKA 2013-10-25 09:13:34 UTC
For -2
* Obsoletes
  - What I said before is meant that Obsoletes lines must
    be changed to "Obsoletes: ruby-openid  <= 2.1.7-11",
    for example (i.e. not = but <=)

* License tag
  - Ah, the license tag for the main package should be
    "Ruby and ASL 2.0 and MIT" (as lib/hmac/hmac.rb,
    lib/openid/yadis/htmltokenizer.rb are under Ruby).
    Please update the license tag again, sorry.

* Permission
  - It seems that some files in -doc subpackage have executable
    permission:

/usr/share/gems/gems/ruby-openid-2.3.0/examples/rails_openid/script/rails
/usr/share/gems/gems/ruby-openid-2.3.0/examples/discover
/usr/share/gems/doc/ruby-openid-2.3.0/rdoc/images/add.png
/usr/share/gems/doc/ruby-openid-2.3.0/rdoc/images/arrow_up.png
/usr/share/gems/doc/ruby-openid-2.3.0/rdoc/images/delete.png
/usr/share/gems/doc/ruby-openid-2.3.0/rdoc/images/tag_blue.png

    At least it seems wrong that png files have executable permission.
    * Note that the executable permission on 
      /examples/rails_openid/script/rails,
      /examples/discover adds the dependency "/usr/bin/env" to -doc
      subpackage. You may also want to suppress this dependency
      (by also removing executable permission on these files, or
       to add filtering macro again (which is contrary to what
       I said before, however for this case it seems okay) )

Comment 8 Ken Dreyer 2013-10-27 22:03:29 UTC
(As a correction to comment #6: my license clarification request is at https://github.com/openid/ruby-openid/issues/64, not 60.)

(In reply to Mamoru TASAKA from comment #7)
>     * Note that the executable permission on 
>       /examples/rails_openid/script/rails,
>       /examples/discover adds the dependency "/usr/bin/env" to -doc
>       subpackage. You may also want to suppress this dependency
>       (by also removing executable permission on these files, or
>        to add filtering macro again (which is contrary to what
>        I said before, however for this case it seems okay) )

For the /usr/bin/env dependency in -doc, I switched that to /usr/bin/ruby. Please let me know if you're ok with this, or if you'd like me to adjust it further (ie. remove it altogether.)

The PNGs' executable bits is a bug in the rdoc gem. I've submitted https://github.com/rdoc/rdoc/pull/258 to fix this.

Here's the newest version.

* Sun Oct 27 2013 Ken Dreyer <ktdreyer> - 2.3.0-3
- Updates for review request (RHBZ #1015778)
- Update obsoletes
- Update license
- Remove /usr/bin/env from -doc auto-requires

Specific changes (in git): http://fedorapeople.org/cgit/ktdreyer/public_git/rubygem-ruby-openid.git/commit/?id=6b384da08eb75cdf95572af52f90393dbaa256ae

Spec: http://ktdreyer.fedorapeople.org/reviews/rubygem-ruby-openid.spec
SRPM: http://ktdreyer.fedorapeople.org/reviews/rubygem-ruby-openid-2.3.0-3.fc21.src.rpm

F21 scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6105416

Comment 9 Mamoru TASAKA 2013-10-28 07:05:47 UTC
(In reply to Ken Dreyer from comment #8)
> The PNGs' executable bits is a bug in the rdoc gem. I've submitted
> https://github.com/rdoc/rdoc/pull/258 to fix this.

Ah, I did not notice this. Would you open a tracker bug for Fedora side?

For this package, approving.

------------------------------------------------------------
   This package (rubygem-ruby-openid) is APPROVED by
   mtasaka
------------------------------------------------------------

Comment 10 Ken Dreyer 2013-10-28 22:48:39 UTC
Thanks for the review!

I've filed bug 1024122 for the PNG images in rubygem-rdoc.

Comment 11 Ken Dreyer 2013-10-28 22:58:31 UTC
New Package SCM Request
=======================
Package Name: rubygem-ruby-openid
Short Description: A library for consuming and serving OpenID identities
Owners: ktdreyer
Branches: f19 f20

Comment 12 Gwyn Ciesla 2013-10-29 11:58:03 UTC
Git done (by process-git-requests).

Comment 13 Fedora Update System 2013-10-29 15:11:20 UTC
rubygem-ruby-openid-2.3.0-3.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/rubygem-ruby-openid-2.3.0-3.fc20

Comment 14 Fedora Update System 2013-10-29 15:12:02 UTC
rubygem-ruby-openid-2.3.0-3.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/rubygem-ruby-openid-2.3.0-3.fc19

Comment 15 Fedora Update System 2013-10-29 18:06:43 UTC
Package rubygem-ruby-openid-2.3.0-3.fc20:
* should fix your issue,
* was pushed to the Fedora 20 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing rubygem-ruby-openid-2.3.0-3.fc20'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2013-20238/rubygem-ruby-openid-2.3.0-3.fc20
then log in and leave karma (feedback).

Comment 16 Fedora Update System 2013-11-08 04:31:00 UTC
rubygem-ruby-openid-2.3.0-3.fc19 has been pushed to the Fedora 19 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 17 Fedora Update System 2013-11-10 06:09:08 UTC
rubygem-ruby-openid-2.3.0-3.fc20 has been pushed to the Fedora 20 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.