Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

stop circular require warnings - guard require "rubygems" #6862

Closed
wants to merge 1 commit into from
Closed

stop circular require warnings - guard require "rubygems" #6862

wants to merge 1 commit into from

Conversation

MSP-Greg
Copy link
Contributor

@MSP-Greg MSP-Greg commented Jan 2, 2019

What was the end-user problem that led to this PR?

Testing rubygems/rubygems with warnings on, specifically, test_gem.rb. Several 'circular require' warnings.

What was your diagnosis of the problem?

RubyGems gets required, then requires Bundler. Several places in Bundler then require RubyGems.

What is your fix for the problem, implemented in this PR?

Guard the RubyGems requires. Note that rubygems/specification is one of the few files loaded by RubyGems, most are autoload. I kept adding guards until all warnings were removed.

Why did you choose this fix out of the possible options?

Unaware of other possible options

@MSP-Greg
Copy link
Contributor Author

MSP-Greg commented Jan 3, 2019

To repo, from RubyGems repo:

rake test RUBYOPT=-v TEST=test/rubygems/test_gem.rb

@MSP-Greg
Copy link
Contributor Author

MSP-Greg commented Jan 3, 2019

ruby/ruby just switched on warnings in their tests. See for similar circular require warnings, starting around line 2912:

https://travis-ci.org/ruby/ruby/jobs/474816395

@segiddins
Copy link
Member

Is this check safe? Is it possible some other file has defined the Gem constant, without all of the classes and methods in there being defined?

@MSP-Greg
Copy link
Contributor Author

MSP-Greg commented Jan 27, 2019

@segiddins

That's a very deep rabbit hole.

What if someone modified $LOAD_PATH so the require statement loaded a different rubygems.rb file?

I haven't checked, but autoload might have similar issues with a improperly defined constant...

I could update the PR where, instead of using the guard, one could do something similar to:

verbose, $VERBOSE = $VERBOSE, false
require "rubygems"
$VERBOSE = verbose

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants