-
-
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
open3.rb broken in JRuby #5018
Comments
@cshupp1 do you know if this is a regression or likely something which has been broken for the whole 9.1.x series? |
@enebo It fails for me with 9.1.8 and 9.0.4.0. Can you reproduce it? |
@cshupp1 we implement capture in terms of popen3 for windows and @headius happens to have a recent PR #4964 which changes popen3 behavior. Not sure how WIP this PR is but maybe you can try that and see if it fixes anything? (I have not started up Windows to look at this yet). We probably have a whole class of errors on windows because MRI has a bunch of native code which we have not yet made the equivalent in jnr-posix. This actually expands out to features of IO in general. We would have worked on this but it is a big job. |
How do I get jruby's complete jar? See this gist: https://gist.github.com/cshupp1/7916b7027d0163a6b7d8ec32643743bf |
@enebo irb will not even come up. Cris |
On:
The results are unchanged.
|
FYI, this does work (Still using improve_windows_open3) (the last one)
|
So for clarity: Both work in MRI. |
This probably should be I'll boot up my Windows VM and see what I can see today. |
Ok, so the only problem I see here is that it's not finding bundle.bat, because the following seems to work fine:
I'll look into the path searching logic, but can you confirm that providing a more specific command makes all your examples work? |
Ok, I've pushed some tweaks to the branch that do the following:
This appears to make your examples all work right. |
Ok, this looks ok in CI and it makes all your examples work, so we'll call it fixed. |
Need this reopened, when trying to generate a war with warbler the dependency on webpacker still fails:
The code referenced is:
|
Is Warbler using JRuby 9.1.16 to generate this war file? It sure looks like it doesn't have my fix. |
Warbler does appear to be using 9.1.16. Consider the following:
Now consider the following irb:
That is unexpected in 9.1.16 right? |
BTW, the bomb out isn't happening in warbler. It is webpacker. |
Hmm...tracing through the logic for Open3.capture3 in JRuby leads me to popen3, popen_run, and then spawn which on Windows goes through our ShellLauncher logic. That logic in turn does appear to be doing the executable search. Indicating that we should do a search:
Proceeding to the search:
The search:
There's also conditional logic that disables the executable search if any of several shell characters ( I should be able to reproduce this on my Windows VM. |
Ok, I don't have a Windows VM yet on this machine. It will take some time to set up. @enebo if you are looking for something to do... |
There were two popen path fixes made on master that never propagated to 9.1: 1925b5d and b0f7aaf. The first uses path searching for popen, and the second fixes a regression in the first (ignoring system PATH). I've cherry-picked them to 9.1 as 686fca1 and 85cb4e9. Fingers crossed, I believe this should fix your issue. |
I have run into this problem on both 9.1.17 and 9.2.5. It does not seem to be fixed. |
@enkessler can you make a little reproducer? I tried several of the things above and I got them to work but our windows process launching stuff is complicated so the specifics of the command string could make a huge difference on figuring out what is wrong. |
@enebo Here's the reproduction repo. Just
|
@enkessler ok I can repro on Win 10. If I change to bundler.bat it works so I feel like we have been here before (in looking at previous comments). Thanks for taking the time to make this. |
@enkessler Looks like we had some reasonable logic commented out which addresses this problem. If we run from .bat I guess we cannot exec but have to run the command within a CMD shell. |
Nice to know that is was an easy fix. Thanks! |
Consider the following:
Now consider MRI ruby
In MRI ruby we can make a call to bundler. JRuby we cannot.
Please note the following in JRuby:
So JRuby should be able to find it.
I have written up the following webpacker bug which shows a workaround, but it just could be that the root cause of the issue is with JRuby.
rails/webpacker#1175
The text was updated successfully, but these errors were encountered: