Fix circular require warning when requiring 'tilt' #221

Merged
merged 1 commit into from Dec 6, 2013

3 participants

@amarshall

A simple require 'tilt' results in the following warning (when Ruby warnings are enabled):

warning: loading in progress, circular require considered harmful

☽ ruby -w -r tilt -e exit
~/.gem/ruby/2.0.0/gems/tilt-2.0.0/lib/tilt/mapping.rb:151: warning: assigned but unused variable - prefix
~/.rubies/ruby-2.0.0-p353/lib/ruby/site_ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55: warning: loading in progress, circular require considered harmful - ~/.gem/ruby/2.0.0/gems/tilt-2.0.0/lib/tilt.rb
    from ~/.rubies/ruby-2.0.0-p353/lib/ruby/site_ruby/2.0.0/rubygems/core_ext/kernel_require.rb:144:in `require'
    from ~/.rubies/ruby-2.0.0-p353/lib/ruby/site_ruby/2.0.0/rubygems/core_ext/kernel_require.rb:135:in `rescue in require'
    from ~/.rubies/ruby-2.0.0-p353/lib/ruby/site_ruby/2.0.0/rubygems/core_ext/kernel_require.rb:135:in `require'
    from ~/.gem/ruby/2.0.0/gems/tilt-2.0.0/lib/tilt.rb:2:in `<top (required)>'
    from ~/.rubies/ruby-2.0.0-p353/lib/ruby/site_ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55:in `require'
    from ~/.rubies/ruby-2.0.0-p353/lib/ruby/site_ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55:in `require'
    from ~/.gem/ruby/2.0.0/gems/tilt-2.0.0/lib/tilt/template.rb:1:in `<top (required)>'
    from ~/.rubies/ruby-2.0.0-p353/lib/ruby/site_ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55:in `require'
    from ~/.rubies/ruby-2.0.0-p353/lib/ruby/site_ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55:in `require'

Since tilt.rb doesn’t actually need Tilt::Template, removing that require has no apparent negative effect and remedies the circular require issue.

@amarshall amarshall Tilt does not explicitly need tilt/template
This fixes a Ruby warning when requiring 'tilt':

> warning: loading in progress, circular require considered harmful
31933f2
@judofyr judofyr merged commit 9857269 into rtomayko:master Dec 6, 2013
@amarshall amarshall deleted the amarshall:circular-require-fix branch Dec 14, 2013
@minad

You should revert this one. It introduces an incompatibility with Tilt 1.3 and Slim. See #198. The warning is not nice but also not relevant. Any idea how to work around it?

@amarshall Breaking backward compatibility is a negative effect.

@minad

You could fix it in the following ugly way:

  • tilt.rb requires tilt/template_class.rb
  • tilt/template.rb only includes tilt.rb
  • tilt/template_class.rb contains the Tilt::Template class
@amarshall

@minad What sort of incompatibility? What steps are required to reproduce it? Can you provide a failing test that exhibits the issue?

@judofyr
Collaborator

@amarshall Previously require 'tilt' was enough to load Tilt::Template.

@minad: What about this? I don't think there's anything that explicitly loads tilt/template that depends on the Tilt module.

diff --git a/lib/tilt.rb b/lib/tilt.rb
index 85b0380..92f2e54 100644
--- a/lib/tilt.rb
+++ b/lib/tilt.rb
@@ -1,4 +1,5 @@
 require 'tilt/mapping'
+require 'tilt/template'

 # Namespace for Tilt. This module is not intended to be included anywhere.
 module Tilt
diff --git a/lib/tilt/template.rb b/lib/tilt/template.rb
index 34dc950..7f4ee05 100644
--- a/lib/tilt/template.rb
+++ b/lib/tilt/template.rb
@@ -1,4 +1,3 @@
-require 'tilt'
 require 'thread'

 module Tilt
@minad

@judofyr Yes, that would be a solution. However someone could have relied on tilt/template.rb to include tilt.rb since the 2.0 release. But at least that would not break anything I know of. So it is fine with me.

@judofyr judofyr added a commit that referenced this pull request Jan 8, 2014
@judofyr judofyr Tweak requiring logic #221:
require 'tilt/template' will now *NOT* require 'tilt'
require 'tilt' will require 'tilt/template'
0f29568
@ademin ademin added a commit to ademin/tilt that referenced this pull request May 19, 2014
@judofyr judofyr Tweak requiring logic #221:
require 'tilt/template' will now *NOT* require 'tilt'
require 'tilt' will require 'tilt/template'
7f5386a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment