Bug 668820

Taking this one.

* Please consider updating to the latest version (3.5.1 atm)

* MUST: The License field in the package spec file must match the actual license.
  - Please review the licensing. At appears that the package is GPLv2 and some
    custom license, where some files are MIT (lib/rdoc/task.rb) or Ruby licensed

* Cleaning
  - %clean section is no longer needed (on Fedora):
    https://fedoraproject.org/wiki/Packaging/Guidelines#.25clean

* Documentation
  - Please do not disable documentation generation, since ruby forces
    installation of ruby-rdoc, therefore rdoc should be available prior the gem
    installation
  - Please consider to provide the documentation in -doc subpackage

* Requires
  - BuildRequires: rubygem(minitest) is needed for text execution

* Tests
  - Please execute test suite using following command:
      ruby -I../lib -e "Dir.glob('test/test_*').each {|t| require t}"
    This allows you to avoid build dependency on rake, hoe, rubyforge and neither
    ZenTest is required IMO.

Btw. I did not mentioned it here before, but according to Ruby Packaging Guidelines: 

"If the same Ruby library is to be packaged for use as a Gem and as a straight Ruby library without Gem support, it must be packaged as a Gem first. To make it available to code that does not use Ruby Gems, a subpackage called ruby-%{gemname} must be created in the rubygem-%{gemname} package such that"

We already have ruby-rdoc package, so this package does not follow the guidelines precisely. I know they should not conflict, they are designed to work side by side, I just believe I should note it here.

What is the status of this bug?

I'm backing off this submission for the time being. I originally thought we needed it for Rails 3 but apparently we do not.

Anyone may feel free to modify and resume this submission. If some cycles free up soon, I'll come back to it.

Hi guys,

It seems that RDoc gem is now Rails 3.0.9 requirement due to updated Rake [1]. I need to reopen and finish this review in order to fix Rails in Rawhide (it is hopefully the last missing piece). Mo, could you please finish the spec? Or is somebody willing to do review, if I am going to finish the .spec?

[1] https://github.com/rails/rails/issues/1598

Vit,
     I took a look at this, and I have to admit I am a bit baffled by the situation.  I looked at the latest rdoc sources (https://github.com/rdoc/rdoc) vs. the current upstream ruby sources (https://github.com/ruby/ruby).  The sources are identical; indeed, there is a commit in the ruby source code (b7528b5edb1f9148ea00ebb6151720e5943b3f0b) that updates the ruby in-tree code to the latest rdoc git.  That seems to say to me that the ruby code is tracking the rdoc, and that the code in the ruby tree is probably the canonical one we should use.
    That being said, then, we already have a ruby-rdoc package that is built out of the ruby SRPM.  Granted, it is a much older version (since Fedora is still on ruby 1.8.7), but it seems to me that we would want to stick with that.  At least, adding another gem that does the same thing as the base ruby library seems to be a recipe for confusion.  Can we not just patch the railties gemspec to remove (or modify) the rdoc dependency, and then just use the ruby-rdoc package we already have?  Is there something else I'm missing here?

Chris lalancette

(In reply to comment #7)
Chris,

You should compare the correct branches, i.e. [1] vs [2]. In that case, you could see that Ruby 1.8 ships with RDoc V1.0.1 - 20041108 and upstream RDoc is currently at 3.8 version. The package which comes bundled with Ruby has name ruby-rdoc while the gem whould have name rubygem-rdoc.

In theory, it would be possible to replace the ruby-rdoc by newer RDoc from the gem, but it would need some magic, because the ruby-rdoc package is not behaving as a gem, i.e. there is no .gemspec file in specifications folder. Since Ruby does not require RubyGems by default you would be in trouble with folder ownership etc.

So at the end, the easiest solution is to let them live side by side. You have never noticed up until now that there are bundled RDoc and RDoc gem, so I am not afraid that this will cause that much confusion.

Of course we could patch Railties to avoid the dependency, but it results in broken Rails. Other solution would be to patch Rake or Rubys RDoc, but if we start with this, it will not be less confusing, since the project will work only on Fedora and not on the other platforms. Please check again the discussion [3] I linked previously for the reasons why the RDoc gem is required by Rails now.

[1] https://github.com/ruby/ruby/blob/ruby_1_8/lib/rdoc/rdoc.rb#L5
[2] https://github.com/rdoc/rdoc/blob/master/lib/rdoc.rb#L107
[3] https://github.com/rails/rails/issues/1598

P.S. It is interesting that in Ruby, there were left bundled RDoc, which is against Fedora policy anyway. Of course it is not the only one library bundled in Ruby.

I want to make change /usr/bin/rdoc to use alternatives mechanize and to give high priority on ones installed by rubygem-rdoc (if this package is going to be reviewed).

(In reply to comment #9)
> I want to make change /usr/bin/rdoc to use alternatives mechanize and to give
> high priority on ones installed by rubygem-rdoc (if this package is going to be
> reviewed).

Could you be more specific? Are you speaking about this [1] alternatives? Because it seems to me that easier way would be to patch the original /usr/bin/rdoc to load the RDoc gem if it is available, otherwise continue with the original RDoc.  E.g. there would be typical RubyGems loader on the top of the /usr/bin/rdoc and if it fails (no RubyGems, no RDoc gem), then it would continue with the original one.


[1] http://fedoraproject.org/wiki/Packaging:Alternatives

(In reply to comment #10)
> (In reply to comment #9)
> > I want to make change /usr/bin/rdoc to use alternatives mechanize and to give
> > high priority on ones installed by rubygem-rdoc (if this package is going to be
> > reviewed).
> 
> Could you be more specific? Are you speaking about this [1] alternatives?

Yes.

> Because it seems to me that easier way would be to patch the original
> /usr/bin/rdoc to load the RDoc gem if it is available, otherwise continue with
> the original RDoc.  E.g. there would be typical RubyGems loader on the top of
> the /usr/bin/rdoc and if it fails (no RubyGems, no RDoc gem), then it would
> continue with the original one.

In this way, people will have to once remove rubygem-rdoc every time they want to use the original rdoc and it seems unpleasant to me.

> [1] http://fedoraproject.org/wiki/Packaging:Alternatives

(In reply to comment #11)
> In this way, people will have to once remove rubygem-rdoc every time they want
> to use the original rdoc and it seems unpleasant to me.

Unpleasant, but IMO:
1) This is the way how it will work if you build/install your own Ruby, so I think Rubyists are used to it. Even now you loose the original rdoc executable once you install the gem version of RDoc.
2) I don't think there will be high demand in using original RDoc once there is installed the gem version. Are you aware about any specific reason why somebody would like to?
3) With RubyGems, there is always possibility to choose the executed version, e.g. you can call "rdoc _3.6_ --version" if you have installed RDoc 3.8 and 3.6 to use RDoc 3.6, so we can do something similar, i.e. rdoc _system_ or rdoc _1.0.1_

(In reply to comment #12)
> (In reply to comment #11)
> > In this way, people will have to once remove rubygem-rdoc every time they want
> > to use the original rdoc and it seems unpleasant to me.
> 
> Unpleasant, but IMO:
> 1) This is the way how it will work if you build/install your own Ruby, 

Sorry but I cannot figure out what you are saying here.

> so I
> think Rubyists are used to it. Even now you loose the original rdoc executable
> once you install the gem version of RDoc.

I only talk about what happens on current Fedora packaging or rpms so I just ignore it when people install gem files outside Fedora's rpm by themselves.

> 2) I don't think there will be high demand in using original RDoc once there is
> installed the gem version. Are you aware about any specific reason why somebody
> would like to?

I don't recall it shortly, however other people may want.

> 3) With RubyGems, there is always possibility to choose the executed version,
> e.g. you can call "rdoc _3.6_ --version" if you have installed RDoc 3.8 and 3.6
> to use RDoc 3.6, 

Again this won't happen on Fedora (please base your talk on Fedora's rpms or Fedora's packaging)

> so we can do something similar, i.e. rdoc _system_ or rdoc
> _1.0.1_

I don't understand your logic here.
- If you want to do rdoc _system_ or so, perhaps we need another patch, and I don't want to do such mess method.
- And specifying rdoc _system_ or so will also require another patches on packages which has /usr/bin/rdoc as BR.

(In reply to comment #13)

ad 1) I was speaking about users who does not use RPM Ruby, e.g. Ruby installed into /opt or using RVM. They will get overridden their default rdoc executable by rubygems one and they don't care.

ad 3) Patch or alternatives, both methods are messy IMO. From my POV, the users expects that they will use the gem version of RDoc as soon as they install the gem. The question is how to fulfill the expectation the best. Also don't forget that any solution we choose will present precedent for future.

So again, could you please shed more light on your alternatives proposal? What version will be default? How the defaults will change, how the user will work with them.

If ruby-rdoc and rubygem-rdoc are both installed on a system, one will need to be given preference over the other for the default tooling. The issue is which one is given preference and the mechanism which to switch between them (if that is supported).

All in all, from the end user's perspective, I imagine the only thing that is important is compatibility, eg if the rdoc version that is installed will correctly handle their ruby documentation needs. 

If the newer rubygem-rdoc won't suffice as a replacement then we should make sure it doesn't step on ruby-rdoc's toes for compatibility reasons. We can always refer to the specific rubygem-rdoc binaries and paths in gems that depend on it (such as rails).

But if the newer rubygem-rdoc gem is fully backwards compatible with ruby-rdoc, then there is no reason to simplify things and just use that by default (falling back to ruby-rdoc if its not present). Care should be taken doing so, we should not refer to any gem bits inside the ruby-rdoc package itself


All in all, this package should be submitted, so lets continue w/ this submission process, resolving this as we go along

(In reply to comment #15)
Mo, I did not payed enough attention to this issue when I did the original review, but re-reviewing the spec again, it might be really the best solution just to ignore/delete the gem rdoc executable as you did in the package, since:

1) It avoids the conflict
2) You can still run the original RDoc
3) The RDoc API which is required by Rails is available using RubyGems.

however

4) You *cannot* easily execute the gem version of RDoc.

But we all should be aware that once the RDoc gem is available, there is nothing which can stop it from being used for generating of documentation during build accidentally.

Updated SPEC: http://mo.morsi.org/files/rpms/rubygem-rdoc.spec
Updated SRPM: http://mo.morsi.org/files/rpms/rubygem-rdoc-3.8-1.fc15.src.rpm
Koji Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=3199490

(In reply to comment #2)
> * Please consider updating to the latest version (3.5.1 atm)

Done, updated to the latest (3.8)


> 
> * MUST: The License field in the package spec file must match the actual
> license.
>   - Please review the licensing. At appears that the package is GPLv2 and some
>     custom license, where some files are MIT (lib/rdoc/task.rb) or Ruby
> licensed
> 
Done. Package includes GPLv2, MIT, and Ruby licenses.


> * Cleaning
>   - %clean section is no longer needed (on Fedora):
>     https://fedoraproject.org/wiki/Packaging/Guidelines#.25clean
> 

Done, clean section removed.


> * Documentation
>   - Please do not disable documentation generation, since ruby forces
>     installation of ruby-rdoc, therefore rdoc should be available prior the gem
>     installation
>   - Please consider to provide the documentation in -doc subpackage
> 

Its seems there is an issue w/ parsing the rubygem-rdoc documentation w/ ruby-rdoc. Whenever I re-enable the --rdoc, I get the following parse error:

Generating HTML...
ERROR:  While generating documentation for rdoc-3.8
... MESSAGE:   Unhandled special: Special: type=17, text="<!-- -->"
... RDOC args: --op /home/mmorsi/rpmbuild/BUILD/rubygem-rdoc-3.8/usr/lib/ruby/gems/1.8/doc/rdoc-3.8/rdoc --main README.txt lib History.txt LICENSE.txt Manifest.txt README.txt RI.txt Rakefile --title rdoc-3.8 Documentation


Grepping the source, it seems the offending line is in History.txt. Since from the rpm spec's perspective installing History.txt and parsing it w/ rdoc is an atomic operation, I've disabled the rdoc generation for the time being.

Added a documentation subpackage, and re-enabled ri generation.



> * Requires
>   - BuildRequires: rubygem(minitest) is needed for text execution
> 

Done


> * Tests
>   - Please execute test suite using following command:
>       ruby -I../lib -e "Dir.glob('test/test_*').each {|t| require t}"
>     This allows you to avoid build dependency on rake, hoe, rubyforge and
> neither
>     ZenTest is required IMO.

./lib/rdoc/task.rb:36 requires 'rake' and 'rake/tasklib'. Had to add rubygem(rake) as Requires and a BuildRequires

Regardless, to get rid of rubygem(hoe) I implemented your suggestion.



(In reply to comment #8)
> P.S. It is interesting that in Ruby, there were left bundled RDoc, which is
> against Fedora policy anyway. Of course it is not the only one library bundled
> in Ruby.

True but you have to recall that rdoc was originally part of the Ruby package then got forked off into the gem. At some point it wouldn't surprise me if it was dropped from ruby internally all together (though this would make it harder for rdoc support in rubygems and what not).



(In reply to comment #16)
> (In reply to comment #15)
> Mo, I did not payed enough attention to this issue when I did the original
> review, but re-reviewing the spec again, it might be really the best solution
> just to ignore/delete the gem rdoc executable as you did in the package, since:

Actually this is what the original submission has. It just removes the bin directory all together.

(In reply to comment #18)
> > * Documentation
> >   - Please do not disable documentation generation, since ruby forces
> >     installation of ruby-rdoc, therefore rdoc should be available prior the gem
> >     installation
> >   - Please consider to provide the documentation in -doc subpackage
> > 
> 
> Its seems there is an issue w/ parsing the rubygem-rdoc documentation w/
> ruby-rdoc. Whenever I re-enable the --rdoc, I get the following parse error:
> 
> Generating HTML...
> ERROR:  While generating documentation for rdoc-3.8
> ... MESSAGE:   Unhandled special: Special: type=17, text="<!-- -->"
> ... RDOC args: --op
> /home/mmorsi/rpmbuild/BUILD/rubygem-rdoc-3.8/usr/lib/ruby/gems/1.8/doc/rdoc-3.8/rdoc
> --main README.txt lib History.txt LICENSE.txt Manifest.txt README.txt RI.txt
> Rakefile --title rdoc-3.8 Documentation
> 
> 
> Grepping the source, it seems the offending line is in History.txt. Since from
> the rpm spec's perspective installing History.txt and parsing it w/ rdoc is an
> atomic operation, I've disabled the rdoc generation for the time being.
> 
> Added a documentation subpackage, and re-enabled ri generation.
> 
Interesting, it seems to me like bug in RubyGems. I have replaced the gem install with following command:

GEM_HOME="%{_builddir}/%{name}-%{version}%{gemdir}" gem install --local \
            -V --force %{SOURCE0}

Which works without any issues and the result should be identical to use of "--install-dir" command line flag.


> (In reply to comment #8)
> > P.S. It is interesting that in Ruby, there were left bundled RDoc, which is
> > against Fedora policy anyway. Of course it is not the only one library bundled
> > in Ruby.
> 
> True but you have to recall that rdoc was originally part of the Ruby package
> then got forked off into the gem. At some point it wouldn't surprise me if it
> was dropped from ruby internally all together (though this would make it harder
> for rdoc support in rubygems and what not).

I didn't know that the RDoc was forked out of Ruby. Interesting. There was discussion that since RubyGems are integrated into R1.9, some parts of stdlib should be moved away from core to gems, but unfortunately, it went in nothing :/

So I have some more nits:

* There should be used %global instead of %define
  - http://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define

* Some files are listed twice:
  warning: File listed twice: /usr/lib/ruby/gems/1.8/gems/rdoc-3.8/History.txt
  warning: File listed twice: /usr/lib/ruby/gems/1.8/gems/rdoc-3.8/LICENSE.txt
  warning: File listed twice: /usr/lib/ruby/gems/1.8/gems/rdoc-3.8/Manifest.txt
  warning: File listed twice: /usr/lib/ruby/gems/1.8/gems/rdoc-3.8/README.txt
  warning: File listed twice: /usr/lib/ruby/gems/1.8/gems/rdoc-3.8/RI.txt
  warning: File listed twice: /usr/lib/ruby/gems/1.8/gems/rdoc-3.8/Rakefile
  - Except the LICENSE.txt, I would move them in to -doc subpackage.
  - May be you just forgot the %dir directive in front of 
    "%{gemdir}/gems/%{gemname}-%{version}/" line? Btw this could be replaced
    shorter %{geminstdir} macro

* rpmlint complains about "macro-in-comment"
  - Please remove the macros from comments.
  - Also, the "--bindir .%{_bindir}" flag for gem installation might help
    to avoid later messing with the bin directory.

I have no other objections at this point. If you fix these issues and nobody else has objections, I will approve this package at Monday.

Maybe simply keeping the "new" rdoc under %gemdir/bin (i.e. /usr/lib/ruby/gems/1.8/bin) but not moving from there to %_bindir is another solution?

(In reply to comment #20)
Yeah, I was also thinking about it, but it would introduce new path which nobody owns (or this package would own, but that is not nice anyway), it is not in path, user does not know and does not expect that the binary is there, so at the end I see more cons against pros.

It seems more favorable to teach ruby community that they can execute the gem by:

ruby -rubygems -e "gem 'rdoc', '>= 0'; load Gem.bin_path('rdoc', 'rdoc', '>=0')"

then to teach them new path.

Since there is not much objections, I APPROVE this package. Mo, please fix the above mentioned nits before importing.

Mamoru, what about using rubygem-rdoc as default for rubygems instead of rdoc? We could also think about marking ruby-rdoc and rubygem-rdoc as conflicting and rubygems then pulls in rubygem-rdoc instead of ruby-rdoc.

May be the gem could also provide ruby(rdoc).

(In reply to comment #23)
> Mamoru, what about using rubygem-rdoc as default for rubygems instead of rdoc?

This sounds reasonable, but:

> We could also think about marking ruby-rdoc and rubygem-rdoc as conflicting and
> rubygems then pulls in rubygem-rdoc instead of ruby-rdoc.

This "conflict" usage is _forbidden_ on Fedora's packaging policy.
https://fedoraproject.org/wiki/Packaging/Conflicts
But for some exceptions, using conflicts needs FPC's approval and I don't think FPC would approve this case.

(In reply to comment #25)
> > We could also think about marking ruby-rdoc and rubygem-rdoc as conflicting and
> > rubygems then pulls in rubygem-rdoc instead of ruby-rdoc.
> 
> This "conflict" usage is _forbidden_ on Fedora's packaging policy.
> https://fedoraproject.org/wiki/Packaging/Conflicts
> But for some exceptions, using conflicts needs FPC's approval and I don't think
> FPC would approve this case.

I am aware of this condition. I'll be thinking about it. May be somebody will help to crystallize this idea ;)

OK, thanks for the ACK. Patch updated to address all outstanding feedback

Updated SPEC: http://mo.morsi.org/files/rpms/rubygem-rdoc.spec
Updated SRPM: http://mo.morsi.org/files/rpms/rubygem-rdoc-3.8-2.fc15.src.rpm
Koji Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=3214663


New Package SCM Request
=======================
Package Name: rubygem-rdoc
Short Description: Produces HTML and command-line documentation for Ruby projects
Owners: mmorsi
Branches:
InitialCC:

Git done (by process-git-requests).

Pushed to rawhide