Skip to content

Update behavior of require_relative#1794

Merged
dbussink merged 2 commits intorubinius:masterfrom
LTe:require_relative_base_path
Jul 5, 2012
Merged

Update behavior of require_relative#1794
dbussink merged 2 commits intorubinius:masterfrom
LTe:require_relative_base_path

Conversation

@LTe
Copy link
Copy Markdown
Contributor

@LTe LTe commented Jul 4, 2012

Ruby MRI raise LoadError with message 'cannot infer basepath from (file)'
when developer try to require_relative in 'irb'.

MRI

lite@LTE-RB ~> irb
1.9.3p125 :001 > require_relative "../file"
LoadError: cannot infer basepath
    from (irb):1:in `require_relative'
    from (irb):1
    from /home/lite/.rvm/rubies/ruby-1.9.3-p125-perf/bin/irb:16:in `<main>'
1.9.3p125 :002 > 

Rubinius before patch

rubinius-2.0.0dev :001 > require_relative "../file"
TypeError: Coercion error: nil.to_str => String failed
    from kernel/common/type.rb:28:in `execute_coerce_to'
    from kernel/common/type.rb:21:in `coerce_to'
    from kernel/common/kernel.rb:28:in `StringValue'
    from kernel/common/type19.rb:42:in `coerce_to_path'
    from kernel/common/file.rb:325:in `dirname'
    from kernel/common/codeloader19.rb:26:in `require_relative'
    from kernel/common/kernel19.rb:176:in `require_relative'
    from (irb):1
    from kernel/common/block_environment.rb:75:in `call_on_instance'
    from kernel/common/eval.rb:72:in `eval'
    from kernel/common/kernel19.rb:42:in `loop'
    from kernel/common/throw_catch19.rb:8:in `catch'
    from kernel/common/throw_catch.rb:10:in `register'
    from kernel/common/throw_catch19.rb:7:in `catch'
    from kernel/common/throw_catch19.rb:8:in `catch'
    from kernel/common/throw_catch.rb:10:in `register'
    from kernel/common/throw_catch19.rb:7:in `catch'
    from kernel/common/codeloader.rb:207:in `require'
    from kernel/common/kernel.rb:631:in `gem_original_require (require)'
    from /home/lite/.rvm/rubies/rbx-head/lib/rubygems/custom_require.rb:36:in `require'
    from kernel/loader.rb:706:in `irb'
    from kernel/loader.rb:845:in `main'rubinius-2.0.0dev :002 >

Rubinius after patch

rubinius-2.0.0dev :001 > require_relative "../file"
LoadError: cannot infer basepath from (irb)
    from kernel/common/codeloader19.rb:26:in `require_relative'
    from kernel/common/kernel19.rb:176:in `require_relative'
    from (irb):1
    from kernel/common/block_environment.rb:75:in `call_on_instance'
    from kernel/common/eval.rb:72:in `eval'
    from kernel/common/kernel19.rb:42:in `loop'
    from kernel/common/throw_catch19.rb:8:in `catch'
    from kernel/common/throw_catch.rb:10:in `register'
    from kernel/common/throw_catch19.rb:7:in `catch'
    from kernel/common/throw_catch19.rb:8:in `catch'
    from kernel/common/throw_catch.rb:10:in `register'
    from kernel/common/throw_catch19.rb:7:in `catch'
    from kernel/delta/codeloader.rb:67:in `load_script'
    from kernel/delta/codeloader.rb:109:in `load_script'
    from kernel/loader.rb:632:in `script'
    from kernel/loader.rb:835:in `main'

@dbussink
Copy link
Copy Markdown
Contributor

dbussink commented Jul 4, 2012

There is a much better way to spec this behavior by looking at what the external behavior is and not by speccing the internal implementation of how the code loader works.

If you run this in a file:

eval("require_relative './file'")

You'll see it will fail the same message. You should be able to use this as a basis for a rubyspec instead of this Rubinius specific spec.

@travisbot
Copy link
Copy Markdown

This pull request fails (merged ff221d0d into db7ebee).

@travisbot
Copy link
Copy Markdown

This pull request passes (merged 693a29dd into db7ebee).

@dbussink
Copy link
Copy Markdown
Contributor

dbussink commented Jul 4, 2012

Can you modify it in such a way that the original specs aren't added and then removed? Right now there is a commit that modifies files in both spec/ruby and another directory, which makes merging to rubyspec harder.

We also like to keep our history clean, so that's another reason to have a nice and clean pull request.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please do not spec the exact error message. We don't enforce them to have the exact same wording, so we don't want to spec the content either then.

@steveklabnik
Copy link
Copy Markdown
Member

Neat! I guess I missed this in my original implementation. :(

@travisbot
Copy link
Copy Markdown

This pull request fails (merged d339c6ef into db7ebee).

@LTe
Copy link
Copy Markdown
Contributor Author

LTe commented Jul 4, 2012

@travisbot
Copy link
Copy Markdown

This pull request passes (merged 977905f0 into db7ebee).

@travisbot
Copy link
Copy Markdown

This pull request passes (merged cc679906 into db7ebee).

@dbussink
Copy link
Copy Markdown
Contributor

dbussink commented Jul 4, 2012

Hmm, maybe I wasn't too clear before. Since this also adds a rubyspec, could you separate it into two commits, one containing the changes to Rubinius, the other the changes to Rubyspec?

@travisbot
Copy link
Copy Markdown

This pull request passes (merged ae997dd into db7ebee).

dbussink added a commit that referenced this pull request Jul 5, 2012
Update behavior of require_relative
@dbussink dbussink merged commit 7ea1598 into rubinius:master Jul 5, 2012
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.

4 participants