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

Do require "rubygems" only when needed #7520

Merged
1 commit merged into from
Dec 25, 2019

Conversation

mame
Copy link
Contributor

@mame mame commented Dec 25, 2019

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

The require causes circular require. See #7505 (comment)

$ touch empty_file

$ RUBYGEMS_GEMDEPS=empty_file ./local/bin/ruby -w -e ''
/home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92: warning: /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92: warning: loading in progress, circular require considered harmful - /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb
	from <internal:gem_prelude>:1:in  `<internal:gem_prelude>'
	from <internal:gem_prelude>:1:in  `require'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb:1417:in  `<top (required)>'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb:1203:in  `use_gemdeps'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/user_interaction.rb:47:in  `use_ui'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb:1204:in  `block in use_gemdeps'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in  `require'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in  `require'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/bundler.rb:11:in  `<top (required)>'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/bundler.rb:11:in  `require_relative'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/bundler/rubygems_integration.rb:3:in  `<top (required)>'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in  `require'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in  `require'

What was your diagnosis of the problem?

Do not require "rubygems" blindly.

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

Add unless defined?(Gem) to make sure it does require "rubygems" only when rubygems is not required.

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

This is needed to release Ruby 2.7.0 so I chose the simplest fix.

This require causes circular require.

```
$ touch empty_file

$ RUBYGEMS_GEMDEPS=empty_file ./local/bin/ruby -w -e ''
/home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92: warning: /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92: warning: loading in progress, circular require considered harmful - /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb
	from <internal:gem_prelude>:1:in  `<internal:gem_prelude>'
	from <internal:gem_prelude>:1:in  `require'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb:1417:in  `<top (required)>'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb:1203:in  `use_gemdeps'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/user_interaction.rb:47:in  `use_ui'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb:1204:in  `block in use_gemdeps'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in  `require'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in  `require'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/bundler.rb:11:in  `<top (required)>'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/bundler.rb:11:in  `require_relative'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/bundler/rubygems_integration.rb:3:in  `<top (required)>'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in  `require'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in  `require'
```
@hsbt
Copy link
Member

hsbt commented Dec 25, 2019

@bundlerbot merge

ghost pushed a commit that referenced this pull request Dec 25, 2019
7520: Do `require "rubygems"` only when needed r=hsbt a=mame

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

The require causes circular require.  See #7505 (comment)

```
$ touch empty_file

$ RUBYGEMS_GEMDEPS=empty_file ./local/bin/ruby -w -e ''
/home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92: warning: /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92: warning: loading in progress, circular require considered harmful - /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb
	from <internal:gem_prelude>:1:in  `<internal:gem_prelude>'
	from <internal:gem_prelude>:1:in  `require'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb:1417:in  `<top (required)>'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb:1203:in  `use_gemdeps'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/user_interaction.rb:47:in  `use_ui'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb:1204:in  `block in use_gemdeps'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in  `require'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in  `require'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/bundler.rb:11:in  `<top (required)>'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/bundler.rb:11:in  `require_relative'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/bundler/rubygems_integration.rb:3:in  `<top (required)>'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in  `require'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in  `require'
```

### What was your diagnosis of the problem?

Do not `require "rubygems"` blindly.

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

Add `unless defined?(Gem)` to make sure it does `require "rubygems"` only when rubygems is not required.

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

This is needed to release Ruby 2.7.0 so I chose the simplest fix.

Co-authored-by: Yusuke Endoh <[email protected]>
@ghost
Copy link

ghost commented Dec 25, 2019

Build succeeded

@ghost ghost merged commit c7c5bce into rubygems:master Dec 25, 2019
@hsbt hsbt added this to the 2.1.3 milestone Dec 25, 2019
deivid-rodriguez pushed a commit that referenced this pull request Jan 1, 2020
7520: Do `require "rubygems"` only when needed r=hsbt a=mame

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

The require causes circular require.  See #7505 (comment)

```
$ touch empty_file

$ RUBYGEMS_GEMDEPS=empty_file ./local/bin/ruby -w -e ''
/home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92: warning: /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92: warning: loading in progress, circular require considered harmful - /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb
	from <internal:gem_prelude>:1:in  `<internal:gem_prelude>'
	from <internal:gem_prelude>:1:in  `require'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb:1417:in  `<top (required)>'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb:1203:in  `use_gemdeps'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/user_interaction.rb:47:in  `use_ui'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb:1204:in  `block in use_gemdeps'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in  `require'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in  `require'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/bundler.rb:11:in  `<top (required)>'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/bundler.rb:11:in  `require_relative'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/bundler/rubygems_integration.rb:3:in  `<top (required)>'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in  `require'
	from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in  `require'
```

### What was your diagnosis of the problem?

Do not `require "rubygems"` blindly.

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

Add `unless defined?(Gem)` to make sure it does `require "rubygems"` only when rubygems is not required.

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

This is needed to release Ruby 2.7.0 so I chose the simplest fix.

Co-authored-by: Yusuke Endoh <[email protected]>
(cherry picked from commit 415e336)
This pull request was closed.
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.

3 participants