Bug 667997 - Review Request: rubygem-rack-mount - Stackable dynamic tree based Rack router
Summary: Review Request: rubygem-rack-mount - Stackable dynamic tree based Rack router
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mo Morsi
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 668008 (view as bug list)
Depends On: 670930 672136
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-01-07 15:59 UTC by Vít Ondruch
Modified: 2011-01-28 13:21 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-01-28 13:21:22 UTC
mmorsi: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)

Description Vít Ondruch 2011-01-07 15:59:23 UTC
Spec URL: http://people.redhat.com/vondruch/rubygem-rack-mount.spec
SRPM URL: http://people.redhat.com/vondruch/rubygem-rack-mount-0.6.13-1.fc14.src.rpm

Description:
Stackable dynamic tree based Rack router. Supports Rack’s X-Cascade convention to continue trying routes if the response returns pass. This allows multiple routes to be nested or stacked on top of each other. Since the application endpoint can trigger the router to continue matching, middle-ware can be used to add arbitrary conditions to any route. This allows you to route based on other request attributes, session information, or even data dynamically pulled from a database.

Comment 1 Minnikhanov 2011-01-07 16:36:29 UTC
*** Bug 668008 has been marked as a duplicate of this bug. ***

Comment 2 Mo Morsi 2011-01-19 00:33:45 UTC
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

* Missing dependency, shouldn't rubygem(rack) be a Requires 

http://rubygems.org/gems/rack-mount

* 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

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

Other than that, looks good, thanks for this

Comment 3 Vít Ondruch 2011-01-24 13:06:40 UTC
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

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

Added missing dependencies

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

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

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

Comment 4 Mo Morsi 2011-01-24 20:49:42 UTC
(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

Comment 5 Vít Ondruch 2011-01-26 13:09:59 UTC
Spec URL: http://people.redhat.com/vondruch/rubygem-rack-mount.spec
SRPM URL:
http://people.redhat.com/vondruch/rubygem-rack-mount-0.6.13-3.fc14.src.rpm


(In reply to comment #4)
> > > * 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.

Huh ... I must be blind :) Fixed. Thank you.

> > 
> > > 
> > > * 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
 
This happened because build failed for the first time (missing RUBYOPT="rubygems" in mock?). It should not ever happen on Koji. Nevertheless, I am now cleaning up the test target directory before the test preparation, so it should not happen anymore.

Thank you for your review.

Comment 6 Vít Ondruch 2011-01-26 13:11:57 UTC
New Package SCM Request
=======================
Package Name: rubygem-rack-mount
Short Description: Stackable dynamic tree based Rack router
Owners: vondruch
Branches:

Comment 7 Jason Tibbitts 2011-01-26 17:56:59 UTC
Git done (by process-git-requests).


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