[Bug 667997] Review Request: rubygem-rack-mount - Stackable dynamic tree based Rack router

[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=667997

Mohammed Morsi <mmorsi at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+

--- Comment #4 from Mohammed Morsi <mmorsi at redhat.com> 2011-01-24 15:49:42 EST ---
(In reply to comment #3)
> Please see updated package:
> 
> Spec URL: http://people.redhat.com/vondruch/rubygem-rack-mount.spec
> SRPM URL:
> http://people.redhat.com/vondruch/rubygem-rack-mount-0.6.13-2.fc14.src.rpm
> 
> (In reply to comment #2)
> > Will take this one
> > 
> > * $ rpmlint rpmbuild/RPMS/noarch/rubygem-rack-mount-*
> > rpmbuild/SRPMS/rubygem-rack-mount-0.6.13-1.fc14.src.rpm | grep -v
> > unexpanded-macro
> >   rubygem-rack-mount.noarch: I: enchant-dictionary-not-found en_US
> >   rubygem-rack-mount.src: W: no-cleaning-of-buildroot %clean
> >   rubygem-rack-mount.src: W: no-buildroot-tag
> >   rubygem-rack-mount.src: W: no-%prep-section
> >   rubygem-rack-mount.src: W: no-%build-section
> >   rubygem-rack-mount.src: W: no-%clean-section
> > 3 packages and 0 specfiles checked; 0 errors, 76 warnings.
> > 
> > Can you add the missing sections (prep, build, clean), the other warnings can
> > be ignored
> > 
> 
> I have added missing sections

Looks good.

> 
> > * Missing dependency, shouldn't rubygem(rack) be a Requires 
> > 
> > http://rubygems.org/gems/rack-mount
> > 
> 
> Added missing dependencies

Looks good.

> 
> > * MUST: Packages must NOT bundle copies of system libraries, rack-mount
> > vendorizes the multimap and regin gems, these need to be separated into their
> > own rpms
> 
> Bundled gems removed and added dependencies instead.

Hrm, the code you added to the specfile doesn't quite accomplish this, instead 
of the "rm -rf gems/..." line you need

"rm -rf %{buildroot}%{geminstdir}/lib/rack/mount/vendor"

I've verified it builds and the test suite still works as intended after this
fix.

> 
> > 
> > * Koji build is green:
> > http://koji.fedoraproject.org/koji/taskinfo?taskID=2730211
> > 
> > * Feel free to tar up the upstream test suite and Rakefile and include and run
> > them in the rpm's check section. Not a requirement for approval though.
> 
> Test suite is executed now during build. Note, however, that the test suite is
> not bundled into resulting package.

Thanks, though I'm getting the following error when I run this:

mkdir /var/tmp/rack-mount-0.6.13
mkdir: cannot create directory `/var/tmp/rack-mount-0.6.13': File exists
error: Bad exit status from /var/tmp/rpm-tmp.3WMbc3 (%check)

The following accomplishes the same thing and works locally

%check
pushd %{buildroot}%{gemdir}
tar xzvf %{SOURCE1} 
ruby -rrubygems -I%{buildroot}%{geminstdir}/lib -I./test /usr/bin/testrb
test/test_*
rm -rf test/
popd


> 
> > Other than that, looks good, thanks for this
> 
> I cannot provide Koji build results as long as the dependencies are not
> satisfied. However, if everything goes well, the package should be prepared
> already.

I've verified it builds in a F14 mock environment w/ the regin and multimap
gems pre installed.


Once you make the two changes above (correct the "rm -rf" command to remove
vendorized libs and fix the %check section so that it doesn't throw error) this
package is 

APPROVED

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