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

Pathname#realpath is returning a String instead of a Pathname #989

Closed
nirvdrum opened this Issue Sep 5, 2013 · 7 comments

Comments

Projects
None yet
3 participants
@nirvdrum
Copy link
Contributor

nirvdrum commented Sep 5, 2013

I grabbed a 1.7.5-dev nightly today (Sept. 4, 2013) and Pathname#realpath in 1.9 mode is returning a String instead of a Pathname. In 1.7.4 it returned a Pathname and it looks like it should, according to the docs. Since it's returning a String, Rails is unable to boot because it tries calling join on the value, expecting a Pathname.

Reproduction case would be any Rails app, I think. I'm running Rails 3.2.13.

@nirvdrum

This comment has been minimized.

Copy link
Contributor Author

nirvdrum commented Sep 5, 2013

IRC discussion:

<atamb0> looks like all it does is delegate realpath to File: https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/ext/pathname/RubyPathname.java#L64
<nirvdrum> I don't really get how that annotation processor works.
<atamb0> and file returns a string
<nirvdrum> I'd have to dig in more to really get what that Java Pathname implementation is doing.
<nirvdrum> I'm surprised this wouldn't cause a rubyspec to fail though.
<nirvdrum> atamb0: Cool.  Then I guess I did understand it :-)
<nirvdrum> atamb0: I guess that means all the other methods mentioned in that delegate call should be double-checked as well.
<atamb0> nirvdrum: maybe just throw this info into the issue and ping @eregon? it seems like the delegating technique is the same thing mri does, but it also always rewraps the string result into a pathname object
@nirvdrum

This comment has been minimized.

Copy link
Contributor Author

nirvdrum commented Sep 5, 2013

Ping @eregon so he can maybe shed some light on the issue.

@ghost ghost assigned BanzaiMan Sep 5, 2013

@BanzaiMan

This comment has been minimized.

Copy link
Member

BanzaiMan commented Sep 5, 2013

Fixed by 090d5dd.

@BanzaiMan BanzaiMan closed this Sep 5, 2013

@eregon

This comment has been minimized.

Copy link
Member

eregon commented Sep 5, 2013

I'm sorry, this was indeed a bug I introduced.
And there seems to be absolutely no spec testing this behavior. I'll add a couple to rubyspec.

@eregon

This comment has been minimized.

Copy link
Member

eregon commented Sep 5, 2013

Added specs in rubyspec/rubyspec@197b812c4730dfb40918f5cf2e70a07f012e67b7

@BanzaiMan

This comment has been minimized.

Copy link
Member

BanzaiMan commented Sep 5, 2013

@eregon FYI: The MRI tests (https://github.com/jruby/jruby/blob/master/test/externals/ruby1.9/pathname/test_pathname.rb) are excluded (https://github.com/jruby/jruby/blob/master/test/externals/ruby1.9/excludes/TestPathname.rb) at the moment. #realpath tests still don't pass, but I haven't looked too deep into it.

@eregon

This comment has been minimized.

Copy link
Member

eregon commented Sep 5, 2013

@BanzaiMan I know, but anyway they do not test if result is an instance of Pathname.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.