Skip to content
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

Circular require considered harmful on faraday/net_http #1234

Closed
rmm5t opened this issue Jan 11, 2021 · 4 comments
Closed

Circular require considered harmful on faraday/net_http #1234

rmm5t opened this issue Jan 11, 2021 · 4 comments

Comments

@rmm5t
Copy link

rmm5t commented Jan 11, 2021

Basic Info

  • Faraday Version: 1.3.0
  • Ruby Version: 2.5.8

Issue description

The require "faraday/net_http" addition in #1222 causes a circular dependency issue.
https://github.com/lostisland/faraday/pull/1222/files#diff-845cc2342ac470a7b749ab1d5f24fa69c5db413a927700e241c5f6c4a67b649aR30

faraday requires faraday/net_http. faraday/net_http requires faraday. wash. rinse. repeat.

/path/vendor/bundle/ruby/2.5.0/gems/faraday-net_http-1.0.0/lib/faraday/net_http.rb:3: warning: loading in progress, circular require considered harmful - /Users/mcgear/work/account-service/vendor/bundle/ruby/2.5.0/gems/faraday-1.3.0/lib/faraday.rb
                from /path/vendor/bundle/ruby/2.5.0/gems/faraday-1.3.0/lib/faraday.rb:30:in  `<top (required)>'
                from /path/vendor/bundle/ruby/2.5.0/gems/faraday-1.3.0/lib/faraday.rb:30:in  `require'
                from /path/vendor/bundle/ruby/2.5.0/gems/faraday-net_http-1.0.0/lib/faraday/net_http.rb:3:in  `<top (required)>'

Steps to reproduce

  1. require "faraday"
@iMacTia
Copy link
Member

iMacTia commented Jan 12, 2021

Interesting, I tested this myself because I wanted to avoid this issue and I didn't notice it:

$ irb
2.7.0 :001 > require 'faraday'
 => true 
$ gem list | grep faraday
  faraday (1.3.0)
  faraday-net_http (1.0.0)

Tested with 2.7 and even 3.0!
Anyway, this is really annoying, but we had to require the adapter from faraday in order to preserve backwards-compatibility, so I guess the fix in this case is to remove the require 'faraday' from the adapter instead.

Once we ship Faraday 2.0, the dependency requirement will swap

@olleolleolle
Copy link
Member

olleolleolle commented Jan 12, 2021

Like this:

$ RUBYOPT=-W2 irb
RUBY_GC_MALLOC_LIMIT=1000000000 (default value: 16777216)
/Users/olle/.rubyrc:25: warning: `&' interpreted as argument prefix
11:11:20 >> require "faraday"
/Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/site_ruby/2.5.0/rubygems/core_ext/kernel_require.rb:54: warning: loading in progress, circular require considered harmful - /Users/olle/.rvm/gems/ruby-2.5.1/gems/faraday-1.3.0/lib/faraday.rb
	from /Users/olle/.rvm/rubies/ruby-2.5.1/bin/irb:11:in  `<main>'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/2.5.0/irb.rb:383:in  `start'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/2.5.0/irb.rb:427:in  `run'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/2.5.0/irb.rb:427:in  `catch'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/2.5.0/irb.rb:428:in  `block in run'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/2.5.0/irb.rb:487:in  `eval_input'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/2.5.0/irb/ruby-lex.rb:231:in  `each_top_level_statement'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/2.5.0/irb/ruby-lex.rb:231:in  `catch'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/2.5.0/irb/ruby-lex.rb:232:in  `block in each_top_level_statement'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/2.5.0/irb/ruby-lex.rb:232:in  `loop'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/2.5.0/irb/ruby-lex.rb:246:in  `block (2 levels) in each_top_level_statement'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/2.5.0/irb.rb:488:in  `block in eval_input'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/2.5.0/irb.rb:623:in  `signal_status'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/2.5.0/irb.rb:491:in  `block (2 levels) in eval_input'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/2.5.0/irb/context.rb:380:in  `evaluate'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/2.5.0/irb/workspace.rb:85:in  `evaluate'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/2.5.0/irb/workspace.rb:85:in  `eval'
	from (irb):1:in  `irb_binding'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/site_ruby/2.5.0/rubygems/core_ext/kernel_require.rb:34:in  `require'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/site_ruby/2.5.0/rubygems/core_ext/kernel_require.rb:130:in  `rescue in require'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/site_ruby/2.5.0/rubygems/core_ext/kernel_require.rb:130:in  `require'
	from /Users/olle/.rvm/gems/ruby-2.5.1/gems/faraday-1.3.0/lib/faraday.rb:30:in  `<top (required)>'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/site_ruby/2.5.0/rubygems/core_ext/kernel_require.rb:54:in  `require'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/site_ruby/2.5.0/rubygems/core_ext/kernel_require.rb:54:in  `require'
	from /Users/olle/.rvm/gems/ruby-2.5.1/gems/faraday-net_http-1.0.0/lib/faraday/net_http.rb:3:in  `<top (required)>'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/site_ruby/2.5.0/rubygems/core_ext/kernel_require.rb:54:in  `require'
	from /Users/olle/.rvm/rubies/ruby-2.5.1/lib/ruby/site_ruby/2.5.0/rubygems/core_ext/kernel_require.rb:54:in  `require'
=> true

@iMacTia
Copy link
Member

iMacTia commented Jan 12, 2021

Fair enough, I'll remove the require from faraday-net_http then 👍

@iMacTia
Copy link
Member

iMacTia commented Jan 12, 2021

faraday-net_http v1.0.1 is now available and should address this 👍
(tested locally by pointing to the main branch before releasing)

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

No branches or pull requests

3 participants