-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
fix quoted paths #3879
fix quoted paths #3879
Conversation
Well, you're on a roll here :-) We have generally recommended that JRuby always be installed in a path without spaces, since there's other Ruby libraries that don't like it. But if we can improve some cases, I'm all for it. |
This seems pretty safe, doesn't it @enebo? Need to do some verification. I'm a little curious why a path is getting in here that has quotes though. Is this a Windows shell quirk of some kind? |
Yeah we must be passing through the quotes as $0 perhaps. I do not see the harm in this though. On unix you can use " in file name but I highly doubt it is valid on windows. Even if it is that would be crazy to use. |
I think I remember reading that On UNIX, the shell would be removing the quotes for us when it parses out arguments. I do feel like there's something else going on here that is the actual root cause. |
@ahorek Maybe you can do a little research to figure out how this quoted string is getting into the runtime? |
Windows CMD does not strip any quoting or process globs (we even have an open bug since we are not handling this stuff on windows). Perhaps we wrote something in launcher/.bat assuming windows would handle this for us? |
@enebo That would make some sense. Maybe all quoted paths passed into our Windows launchers are wrong? |
@headius |
@ahorek Ahh that is interesting. So perhaps this quote-stripping is something we actually do need to do. We should look at MRI's logic for process-launching on Windows and see if they strip this stuff too. |
I did not see any logic at a glance, but there's a lot of win32-specific process stuff. |
fixes #3878