Bug 541491

Some notes:

* Explicit version dependency
  - ">= 3.0" on Requires: rubygem(sexp_processor) is redundant
    because all rubygem-sexp_processor shipped on Fedora satisfies
    this version dependency:
    https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires

* %check
----------------------------------------------------------------
    16  # These test cases are carried in the ParseTree gem in test/. Carry them here
    17  # rather than attempting to install ParseTree-doc in check and introducing a circular
    18  # dependency
    19  Source1: pt_testcase.rb
    79  %check
    80  pushd .%{geminstdir}
    81  cp %{SOURCE1} test/
    82  rake test
----------------------------------------------------------------
  - IMO if this script is really needed for "rake test" (and actually
    it seems so), this script should also be included in the rebuilt
    binary rpm (i.e. better to move the lines 80-81 to %build).

? Dependency loop
  - lib/gauntlet_rubyparser.rb contains:
----------------------------------------------------------------
     8  require 'rubygems'
     9  require 'ruby2ruby'
    10  require 'ruby_parser'
    11  
    12  require 'gauntlet'
----------------------------------------------------------------
    i.e. this script needs two other gems: "ruby2ruby" "gauntlet"
    - The formar one causes dependency loop
    - The latter one is not found on Fedora (even on review request)
    Can this dependency (rather, this script) be ignored?

Thank you for the review.

(In reply to comment #1)
> Some notes:
> 
> * Explicit version dependency
>   - ">= 3.0" on Requires: rubygem(sexp_processor) is redundant
>     because all rubygem-sexp_processor shipped on Fedora satisfies
>     this version dependency:
>     https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires
> 

Noted, thanks.

> * %check
> ----------------------------------------------------------------
>     16  # These test cases are carried in the ParseTree gem in test/. Carry
> them here
>     17  # rather than attempting to install ParseTree-doc in check and
> introducing a circular
>     18  # dependency
>     19  Source1: pt_testcase.rb
>     79  %check
>     80  pushd .%{geminstdir}
>     81  cp %{SOURCE1} test/
>     82  rake test
> ----------------------------------------------------------------
>   - IMO if this script is really needed for "rake test" (and actually
>     it seems so), this script should also be included in the rebuilt
>     binary rpm (i.e. better to move the lines 80-81 to %build).
> 

Good idea.

> ? Dependency loop
>   - lib/gauntlet_rubyparser.rb contains:
> ----------------------------------------------------------------
>      8  require 'rubygems'
>      9  require 'ruby2ruby'
>     10  require 'ruby_parser'
>     11  
>     12  require 'gauntlet'
> ----------------------------------------------------------------
>     i.e. this script needs two other gems: "ruby2ruby" "gauntlet"
>     - The formar one causes dependency loop
>     - The latter one is not found on Fedora (even on review request)
>     Can this dependency (rather, this script) be ignored?  

Ah you noticed this as well, I should have added some notes inline.

I chose to ignore it's dependencies as the developer didn't add it as either a primary or development dependency in the Rakefile. It's also not a library as far as I can tell, more of a script used in testing, and not included/invoked in any of the unit testing.

Plus looking at the gauntlet gem itself it's clearly geared toward library development rather than providing functionality to a library.

What's the correct approach here, %exclude the script? Add a note about it and leave dependencies as is?

Will go on the assumption you'd rather it be excluded.

Spec URL: http://magoazul.com/wip/SPECS/rubygem-ruby_parser.spec
SRPM URL: http://magoazul.com/wip/SRPMS/rubygem-ruby_parser-2.0.4-2.fc13.src.rpm

* Sun Nov 29 2009 Matthew Kent <mkent> - 2.0.4-2
- Move pt_testcase.rb to the build stage so it's included in the rpm (#541491).
- Drop version requirements for sexp_processor as it is a new package
  (#541491).
- Exclude gauntlet_rubyparser.rb as it introduces a circular dependency
  (#541491).

(In reply to comment #2)
> What's the correct approach here, %exclude the script? Add a note about it and
> leave dependencies as is?
> 
> Will go on the assumption you'd rather it be excluded.  

- If you are sure that no one would need this script, then simply
  exclude the script. However if there may be some people who wants
  this script, rather leave this script as it is (and also leave
  the dependency as it is). Those who want to use this script can
  install the needed dependency by him/herself
  ( note that I don't know how this script is to be used, so I would
    keep this script as it is )

Spec URL: http://magoazul.com/wip/SPECS/rubygem-ruby_parser.spec
SRPM URL:
http://magoazul.com/wip/SRPMS/rubygem-ruby_parser-2.0.4-2.fc13.src.rpm
(In reply to comment #4)
> (In reply to comment #2)
> > What's the correct approach here, %exclude the script? Add a note about it and
> > leave dependencies as is?
> > 
> > Will go on the assumption you'd rather it be excluded.  
> 
> - If you are sure that no one would need this script, then simply
>   exclude the script. However if there may be some people who wants
>   this script, rather leave this script as it is (and also leave
>   the dependency as it is). Those who want to use this script can
>   install the needed dependency by him/herself
>   ( note that I don't know how this script is to be used, so I would
>     keep this script as it is )  

Yeah I suppose I can't be certain no one would use it. I'll leave it in.

Spec URL: http://magoazul.com/wip/SPECS/rubygem-ruby_parser.spec
SRPM URL: http://magoazul.com/wip/SRPMS/rubygem-ruby_parser-2.0.4-3.fc13.src.rpm

* Mon Nov 30 2009 Matthew Kent <mkent> - 2.0.4-3
- Remove exclude for gauntlet_rubyparser.rb, let user deal with dependencies if
  they need it.

One comment:
-----------------------------------------------------
+# Script creates a circular dependency and is primarily for development
+# Included, but it's dependencies aren't met.
+%{geminstdir}/lib/gauntlet_rubyparser.rb
-----------------------------------------------------
- This %files entry causes duplicate %files entry:
-----------------------------------------------------
   120  warning: File listed twice: /usr/lib/ruby/gems/1.8/gems/ruby_parser-2.0.4/lib/gauntlet_rubyparser.rb
-----------------------------------------------------
  as this file is already included in
-----------------------------------------------------
%{geminstdir}/lib
-----------------------------------------------------
  Just leave the line as a comment and avoid duplicate %files
  entry.

--------------------------------------------------------------
  This package (rubygem-ruby_parser) is APPROVED by mtasaka
--------------------------------------------------------------

One more thing
* Timestamp
  - Use "cp -p" instead of just "cp" to keep timestamps on installed
    files:
    https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps

Still approved.

New Package CVS Request
=======================
Package Name: rubygem-ruby_parser
Short Description: A ruby parser written in pure ruby
Owners: mkent
Branches: F-11 F-12
InitialCC:

cvs done.

rubygem-ruby_parser-2.0.4-3.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/rubygem-ruby_parser-2.0.4-3.fc11

rubygem-ruby_parser-2.0.4-3.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/rubygem-ruby_parser-2.0.4-3.fc12

Closing

rubygem-ruby_parser-2.0.4-3.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

rubygem-ruby_parser-2.0.4-3.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: rubygem-ruby_parser
New Branches: el5 el6
Owners: stahnma

mkent is aware of request.

Git done (by process-git-requests).