Skip to content

Do not require rdoc from rdoc.gemspec #505

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 31, 2017

Conversation

rhenium
Copy link
Member

@rhenium rhenium commented Aug 29, 2017

Use require_relative instead.

When running under a Bundler environment (e.g., 'bundle exec rake -T'),
RubyGems first activates the RDoc installed globally through the require
statement in rdoc.gemspec. This is problematic since Bundler then tries
to activate the gemspec defined in rdoc.gemspec. They don't match and
this causes an error like this:

bundler: failed to load command: rake (/tmp/rdoc/.bundle/gems/ruby/2.5.0/bin/rake)
Gem::LoadError: You have already activated rdoc 5.1.0, but your Gemfile requires rdoc 6.0.0.beta1. Since rdoc is a default gem, you can either remove your dependency on it or try updating to a newer version of bundler that supports rdoc as a default gem.
  /opt/ruby/trunk/lib/ruby/gems/2.5.0/gems/bundler-1.15.4/lib/bundler/runtime.rb:317:in `check_for_activated_spec!'
  /opt/ruby/trunk/lib/ruby/gems/2.5.0/gems/bundler-1.15.4/lib/bundler/runtime.rb:32:in `block in setup'
  /opt/ruby/trunk/lib/ruby/2.5.0/forwardable.rb:229:in `each'
  /opt/ruby/trunk/lib/ruby/2.5.0/forwardable.rb:229:in `each'
  /opt/ruby/trunk/lib/ruby/gems/2.5.0/gems/bundler-1.15.4/lib/bundler/runtime.rb:27:in `map'
  /opt/ruby/trunk/lib/ruby/gems/2.5.0/gems/bundler-1.15.4/lib/bundler/runtime.rb:27:in `setup'
  /opt/ruby/trunk/lib/ruby/gems/2.5.0/gems/bundler-1.15.4/lib/bundler.rb:101:in `setup'
  /opt/ruby/trunk/lib/ruby/gems/2.5.0/gems/bundler-1.15.4/lib/bundler/setup.rb:9:in `<top (required)>'
  /opt/ruby/trunk/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:60:in `require'
  /opt/ruby/trunk/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:60:in `require'

Use require_relative instead.

When running under a Bundler environment (e.g., 'bundle exec rake -T'),
RubyGems first activates the RDoc installed globally through the require
statement in rdoc.gemspec. This is problematic since Bundler then tries
to activate the gemspec defined in rdoc.gemspec. They don't match and
this causes an error like this:

	bundler: failed to load command: rake (/tmp/rdoc/.bundle/gems/ruby/2.5.0/bin/rake)
	Gem::LoadError: You have already activated rdoc 5.1.0, but your Gemfile requires rdoc 6.0.0.beta1. Since rdoc is a default gem, you can either remove your dependency on it or try updating to a newer version of bundler that supports rdoc as a default gem.
	  /opt/ruby/trunk/lib/ruby/gems/2.5.0/gems/bundler-1.15.4/lib/bundler/runtime.rb:317:in `check_for_activated_spec!'
	  /opt/ruby/trunk/lib/ruby/gems/2.5.0/gems/bundler-1.15.4/lib/bundler/runtime.rb:32:in `block in setup'
	  /opt/ruby/trunk/lib/ruby/2.5.0/forwardable.rb:229:in `each'
	  /opt/ruby/trunk/lib/ruby/2.5.0/forwardable.rb:229:in `each'
	  /opt/ruby/trunk/lib/ruby/gems/2.5.0/gems/bundler-1.15.4/lib/bundler/runtime.rb:27:in `map'
	  /opt/ruby/trunk/lib/ruby/gems/2.5.0/gems/bundler-1.15.4/lib/bundler/runtime.rb:27:in `setup'
	  /opt/ruby/trunk/lib/ruby/gems/2.5.0/gems/bundler-1.15.4/lib/bundler.rb:101:in `setup'
	  /opt/ruby/trunk/lib/ruby/gems/2.5.0/gems/bundler-1.15.4/lib/bundler/setup.rb:9:in `<top (required)>'
	  /opt/ruby/trunk/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:60:in `require'
	  /opt/ruby/trunk/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:60:in `require'

As a bonus, unnecessary magic comment is removed.
RubyGems' rubygems/test_case tries to activate 'json' gem explicitly.
@aycabta
Copy link
Member

aycabta commented Aug 29, 2017

I worried about RDoc documentation step in CRuby building with this patch, but I verified what it works fine.

# -*- encoding: utf-8 -*-
$:.unshift File.expand_path("../lib", __FILE__)
require 'rdoc'
require_relative "lib/rdoc"
Copy link
Member

@aycabta aycabta Aug 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost string literals for require in RDoc are with single-quote. The command below is for listing require with double-quote:

$ git grep '^require "'           
bin/console:require "bundler/setup"                        
bin/console:require "rdoc"                                 
bin/console:require "irb"                                  
lib/rdoc/ruby_lex.rb:require "e2mmap"                      
lib/rdoc/ruby_lex.rb:require "irb/slex"                    
lib/rdoc/ruby_lex.rb:require "stringio"                    
test/test_rdoc_parser_ruby.rb:require "\#{prefix}/a_library"

The bin/console is auto-generated code as you know, the lib/rdoc/ruby_lex.rb is forked source code from IRB and last one is dynamic string. So I think what you should use single-quote for this require.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose double-quotes following the existing string literals in rdoc.gemspec. Do you want me to replace them all?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I explained incompletely. The double quotes for require are stemmed from historical background. I just wanted to say about most require are used with single quote. But it's not important if I really think about it.

@drbrain
Copy link
Member

drbrain commented Aug 30, 2017

I now think double quotes for most strings is best, but I know it breaks tradition with old rdoc code.

@aycabta
Copy link
Member

aycabta commented Aug 31, 2017

OK, I understand this problem and I sometimes encountered the same situation with Bunlder. This is important fixing.

The string literal type is minor important in the now.

@aycabta aycabta merged commit 7c417a1 into ruby:master Aug 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants