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.

Sign up for GitHub

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

Jump to bottom

Added uri to path when using file protocol to chdir #3127

Merged
merged 2 commits into from
Jul 17, 2015
Merged

Added uri to path when using file protocol to chdir #3127

merged 2 commits into from
Jul 17, 2015
+3 −1

Conversation

jkutner
Copy link
Member

@jkutner jkutner commented Jul 10, 2015

@jkutner jkutner mentioned this pull request Jul 10, 2015
Closed
mkristian
mkristian reviewed Jul 10, 2015
View reviewed changes
core/src/main/java/org/jruby/RubyDir.java
@@ -326,6 +326,9 @@ public static IRubyObject chdir(ThreadContext context, IRubyObject recv, IRubyOb
if (adjustedPath.startsWith("uri:")){
realPath = adjustedPath;
}
else if (adjustedPath.startsWith("file:")){
realPath = "uri:" + adjustedPath;
}
Copy link
Member

Choose a reason for hiding this comment

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

a path with !/ is treated as jar uri whether this is a file: or jar: or jar:file: prefix or not. and this is the case where you want the uri: to add. but for uri: to work properly it needs new URL(adjustedPath).openStream() to work. i.e. in your case make sure there is jar:file: prefix

an path with file: onto the filesystem without the !/ better just remove the file: prefix and let jruby use the filesystem.

but I do not understand why your current patch fixes the warbler problem

Copy link
Member

Choose a reason for hiding this comment

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

I think we all to chdir to all paths following this pattern: https://github.com/jruby/jruby/blob/jruby-1_7/core/src/main/java/org/jruby/RubyFile.java#L670

it feels wrong to switch one pseudo-uri to another

@jkutner
Copy link
Member Author

jkutner commented Jul 10, 2015

No, neither 1.7.21 nor 1.7.22.dev fixed this. I tried removing the file: prefix, and got this error:

java.net.MalformedURLException: no protocol: /Users/jkutner/workspace/warbler-debug/warbler-debug.war!/WEB-INF/gems/bundler/gems/ruby-units-a777aeebcfbd/lib/ruby_units/version.rb
    at java.net.URL.<init>(URL.java:586)
    at java.net.URL.<init>(URL.java:483)
    at java.net.URL.<init>(URL.java:432)
    at org.jruby.util.URLResource.createRegularURI(URLResource.java:199)
    at org.jruby.util.URLResource.create(URLResource.java:187)
    at org.jruby.util.JRubyFile.createResource(JRubyFile.java:91)
    at org.jruby.util.JRubyFile.createResource(JRubyFile.java:102)
    at org.jruby.util.JRubyFile.createResource(JRubyFile.java:75)
    at org.jruby.RubyFileStat.setup(RubyFileStat.java:122)
    at org.jruby.RubyFileStat.newFileStat(RubyFileStat.java:97)
    at org.jruby.Ruby.newFileStat(Ruby.java:3342)
    at org.jruby.RubyFile.stat(RubyFile.java:894)

I will investigate the .jrubydir

@mkristian
Copy link
Member

the .jrubydir will not help here with this concrete bundler problem. just passing on the realPath = adjustedPath; should help and keeps things more consistent.

yes, I did not run the sample repo at the beginning.

@jkutner
Copy link
Member Author

jkutner commented Jul 10, 2015

Ah, i understand now. I will improve this

mkristian added a commit that referenced this pull request Jul 17, 2015
@mkristian
Merge pull request #3127 from jkutner/jruby-1_7
26d0b20 
Added uri to path when using file protocol to chdir
@mkristian mkristian merged commit 26d0b20 into jruby:jruby-1_7 Jul 17, 2015
@mkristian
Copy link
Member

never saw the update - but saw your question on IRC :)

@jkutner
Copy link
Member Author

jkutner commented Jul 17, 2015

@mkristian thanks for your help with this!

@enebo enebo modified the milestone: Non-Release May 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Non-Release
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jkutner @mkristian @enebo