Improvements for popen process launch in Windows #6532
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a process launch is attempted against a non-existant file, Windows JDK will raise an IOException that includes both this message and some information about the win32 CreateProcess call. We check endsWidth to recognize this as a ENOENT style error.
This modifies popen logic to use the same executable searching and shell handling that the system call uses, in order to properly search for the target executable. See jruby#6516
|
This change is a bit of a shot in the dark, since our testing on Windows is a little weak. I was able to execute the early stages of @enebo's verification script, so things seem to be working, but a more complete verification is warranted. |
|
This has been verified by me testing basic gem installs and @enebo running his pre-release Windows verification scripts that build and run a Rails app. We'll go with it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The primary fix here is to do the same argument processing and executable searching for
popenthat we do forsystemby aligning the relevant logic. The other fix is to handle another special case JDK error when launching a process using a path that does not exist.This will fix #6516 once finished.