This service will be undergoing maintenance at 00:00 UTC, 2017-10-23 It is expected to last about 30 minutes
Bug 458139 - (ruby-pam) Review Request: ruby-pam - Ruby bindings for pam
Review Request: ruby-pam - Ruby bindings for pam
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Darryl L. Pierce
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-08-06 12:44 EDT by Bryan Kearney
Modified: 2015-06-21 20:06 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-10-24 08:50:17 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
dpierce: fedora‑review+
tkuratom: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Bryan Kearney 2008-08-06 12:44:40 EDT
Spec URL: http://www.thincrust.net/download/ruby-pam.spec
SRPM URL: http://www.thincrust.net/download/ruby-pam-1.5.2-1.fc9.src.rpm
Description: Ruby bindings for pam. 

This is my first package and I will require sponsorship.
Comment 1 Darryl L. Pierce 2008-08-13 10:23:18 EDT
1. Source0 should be the full path for getting the source package, something like: http://sourceforge.net/projects/ruby-pam/%{name}-%{version}.tgz

2. Source0 should use variables for the version (see above).
Comment 2 Bryan Kearney 2008-08-19 08:45:07 EDT
I have made the changes per your suggestions. Please re-verify:

pec URL: http://www.thincrust.net/download/rubygem-pam.spec
SRPM URL: http://www.thincrust.net/download/rubygem-pam-1.5.2-2.fc9.src.rpm
Description: Ruby bindings for pam.
Comment 3 Darryl L. Pierce 2008-08-21 15:37:23 EDT
* General

 XX - Package name
 
 The secondary package delivering the binary library should be delivered in a package named "ruby-pam-lib".
   
 XX - License info is accurate
 XX - License tag is correct and licenses are approved
 
The website says LGPL without version.
The spec says LGPLv2+.
The COPYING file in the GEM lists the original authors name with no mention of LGPL.
 
 OK - License files are installed as %doc
 OK - Specfile name
 OK - Specfile is legible
 OK - No prebuilt binaries included
 OK - BuildRoot value (one of the recommended values)
 OK - PreReq not used
 OK - Source md5sum matches upstream
 OK - No hardcoded pathnames
 OK - Package owns all the files it installs
 OK - 'Requires' create needed unowned directories
 OK - Package builds successfully on i386 and x86_64 (mock)
 OK - BuildRequires sufficient
 OK - File permissions set properly
 OK - Macro usage is consistent
 OK - rpmlint is silent

* Package a rubygem

 OK - Package is named rubygem-%{gemname}
 XX - Source points to full URL of gem
 
 Source0 is just the filename, not the full URL for downloading the source.
 
 OK - Package version identical with gem version
 XX - Package Requires and BuildRequires rubygems
 
 No "Requires: rubygems" in the spec.
 
 OK - Package provides rubygem(%{gemname}) = %version
 OK - Package requires gem dependencies correctly
 OK - %prep and %build are empty
 OK - %gemdir defined properly, and gem installed into it
 OK - Package owns its directories under %gemdir

** noarch rubygem

 OK - No arch-specific content in %{gemdir}
 OK - Package is noarch

** arch rubygem

 OK - No arch specific content in %{gemdir}
 OK - Defines ruby_sitearch from rbconfig
 OK - arch specific content moved to %{ruby_sitearch}
Comment 4 Bryan Kearney 2008-08-22 14:08:40 EDT
Synced up license into. Changed the package name to be in line with the Ruby gem packaging stanhdards, added the requires. 


New Spec file is here: http://ruby-pam.rubyforge.org/git?p=ruby-pam.git;a=blob;f=rubygem-pam.spec;

New Source RPM: http://rubyforge.org/frs/download.php/41730/rubygem-pam-1.5.2-2.fc9.src.rpm
Comment 5 Bryan Kearney 2008-09-08 14:29:53 EDT
Newest Version.

Ensured valid license and issue with source code / source rpm.

Spec File: http://rubyforge.org/frs/download.php/42936/rubygem-pam.spec
Source RPM: http://rubyforge.org/frs/download.php/42933/rubygem-pam-1.5.2-2.fc9.src.rpm
Comment 6 Darryl L. Pierce 2008-09-12 07:56:00 EDT
There are several warnings reporting by rpmlint on both rubygem-pam and ruby-pam that need to be resolved.

 XX - License tag is correct and licenses are approved

rpmlint is complaining about LGPL as an invalid license.

 XX - Source points to full URL of gem

 Source0 needs to use the %{name}, %{version} and %{release} macros rather than a hard-coded filename.

Also, try running a scratch build locally:

     koji build --scratch local rubygem-pam-1.5.2-2.fc9.src.rpm

and make sure there are no build errors.
Comment 7 Bryan Kearney 2008-09-12 15:35:14 EDT
* License was changed to LGPLv2+. The source website shows LGPL (which is all I can do) but the LICENS file shows LGPL 2.1

* rpmlint passes on the source rpm, with no warning
* rpmline shows no-documentation and soft-link warnings on the rpms. They install and run fine.
* Source0 now uses the macros
* Builds fine in koji for i386 and x86_64 (http://koji.fedoraproject.org/koji/taskinfo?taskID=823198)
Comment 8 Darryl L. Pierce 2008-09-15 15:49:22 EDT
Please post the updated SPEC and SRPM. I'll test and, if all works, I'll go ahead and approve the package.
Comment 10 Darryl L. Pierce 2008-09-16 11:27:27 EDT
APPROVED.
Comment 11 David Lutterkort 2008-09-22 13:36:06 EDT
There's something not right with this specfile. When I build it, I get a file

  /usr/lib/ruby/gems/1.8/gems/pam-1.5.2/lib/pam.so

in the x86_64 RPM; that goes directly against the 'no arch-specific content in %gemdir'

Why is this file even needed ? You already put the versioned .so into %ruby_sitearch, and the ruby-pam RPM puts an unversioned link into %ruby_sitearch
Comment 12 Bryan Kearney 2008-09-23 09:10:23 EDT
Thank you for the review!

I tested this by removing the link in /usr/lib/ruby/gems/1.8/gems/pam-1.5.2/lib/pam.so

and when launching with irb -rubygems I got an error when I attempted to  require pam.

I then modified the library itself to be pam.so instead of pam.1.5.2.so. This allowed the gem to install.... but also allowed this to work

irb -rpam 

with only the gem install which I believe is incorrect. So, if the usage of the softlink in the gem directory to the arch directory is not allowed then I will look to add a single ruby file which loads up the versioned library. This seems like the best solution of meeting the rpm needs and the gem needs. Any concerns with this approach?
Comment 13 Bryan Kearney 2008-09-23 16:22:00 EDT
I have added a shim layer to the gem which in turn loads _pam which should pick up the libary from the arch load path. I followed the augeas pattern.

As an aside, should this pattern be put into the packaging guidelines?

New Spec File: http://bkearney.fedorapeople.org/rubygem-pam-1.5.3-1.fc9.src.rpm
New SRPM: http://bkearney.fedorapeople.org/rubygem-pam.spec

koji build clean
one rpmlint warning for nodoc on ruby-pam (doc is in rugygem-pam)
Comment 14 Darryl L. Pierce 2008-10-17 08:53:52 EDT
All is well with me. Approved.
Comment 15 Bryan Kearney 2008-10-17 08:58:18 EDT
New Package CVS Request
=======================
Package Name: ruby-pam
Short Description: PAM bindings for ruby
Owners: bkearney
Branches: F-9 F10
InitialCC: None
Comment 16 Kevin Fenzi 2008-10-19 18:16:23 EDT
cvs done.
Comment 17 Bryan Kearney 2008-10-20 09:51:06 EDT
Package Rename CVS Request
=======================
Package Name: rubygem-pam
Short Description: PAM bindings for ruby
Owners: bkearney
Branches: F-9 F10
InitialCC: None

Sorry...goofed the package name. Should have been rubygem-pam

-- bk
Comment 18 Bryan Kearney 2008-10-22 10:49:55 EDT
Package Change Request
======================
Package Name: ruby-pam
New Branches: None
Owners:  bkearney

The original package name was incorrect. It should be renamed rubygem-pam.
Comment 19 Toshio Kuratomi 2008-10-23 16:21:31 EDT
cvs done.

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