Handle exec error #722
Handle exec error #722
Conversation
|
The docs are at https://github.com/actions/toolkit/tree/main/packages/exec The try/catch works, but might hide other issues which we shouldn't necessarily ignore (e.g. passing bad arguments to that function). |
|
@eregon I'll try. I don't know the syntax for that but I'll figure it out. |
Apparently this exec errors when the subprocess fails rather than returning an error code. This causes the whole setup process to terminate. This patch catches the error and uses that to indicate failure to launch.
headius
force-pushed
the
handle_jruby_exec_error
branch
from
3224696 to
82eb255
Compare
|
@eregon In theory the latest attempt should do it. TruffleRuby failure seems very much unrelated. |
|
Sorry about the churn here but without a better way to test this we're forced to iterate in the wild. Does GHA have any test instance or harness that can be used off the grid? |
|
There is https://github.com/nektos/act but I haven't tried it. |
|
@eregon I pushed a throw-away commit to test with a bad flag. |
|
It seems to hang on Windows, see https://github.com/ruby/setup-ruby/actions/runs/13841525436/job/38730124830?pr=722 and https://github.com/ruby/setup-ruby/actions/runs/13841525436/job/38730135071?pr=722 |
|
Is there something else the JRuby install on Windows does to make it runnable? I literally used the same code you had for the final version print-out and it did not hang there. |
|
Yeah, so it seems a JRuby 10 + Windows bug. |
|
I think at this point we won't find a quick fix, it sounds like a difference between JRuby 9 and 10. |
|
@eregon The bat file is not the preferred way to launch on Windows and may introduce other problems. Why is it running with the bat file? |
|
We are looking into the |
|
I don't know, that may be an issue of exec.exec, or the builds of JRuby on Windows. Example build of a jruby release build: https://github.com/ruby/ruby-builder/actions/runs/13047231536/job/36399785028#step:9:7 I misremembered about jruby-launcher, that's only done for releases and on linux/macOS, e.g. |
|
JRuby's dist (tar.gz and zip)includes a jruby.exe in bin out of the box. We cannot reproduce the hang on a local Windows system. |
|
See jruby/jruby#6042 on the reasoning for adding jruby.bat |
|
I will try something here as a test. No way to verify these things locally and it does not hang with any combination of 10 build and ruby.bat for us. |
|
Feel free to try changing https://github.com/ruby/jruby-dev-builder/blob/a4cdaa60a48c4bfb237f6071f950752ec59ec6e0/.github/workflows/build.yml#L129-L130 since anyway jruby-head on Windows is broken. I'll log off now, it's quite late here. |
|
I will be forced to undo the 9.4 to 10 change if you will be unable to merge anything further here, but I will also be unable to fix anything here. |
|
Switching to the exe did not help either so it's not that. We are trying to find a way to reproduce the issue on Windows. I will keep trying things here but will revert the jruby-head change before giving up for the day. |
headius
force-pushed
the
handle_jruby_exec_error
branch
2 times, most recently
from
99042bc to
a2bc55b
Compare
|
I have fixed the Windows hangs by bypassing the launcher altogether. We have no explanation for it hanging, as both the tarball and a local HEAD build on Windows 10 both work fine, with either a good or bad JDK. Bypassing the launcher for this JRuby-specific logic is also probably better; it's fewer processes to launch and avoids variability in the launcher behavior. |
Native launcher appears to hang on GHA when the JDK version is too old for JRuby 10. This bypasses it and should be a bit lighter. It will also avoid any variability in the launcher used, since Unixes and Windows already use different executables.
headius
force-pushed
the
handle_jruby_exec_error
branch
from
a2bc55b to
31ccf06
Compare
|
All tests are green with none hanging at 31ccf06. |
|
OK, quite weird, but this solution seems alright, merging. |
|
Is the fix released then? I just started a new jruby-dev-builder release but cancelled it when I saw you merged. |
|
Yes, it's released, the passing CI green here means everything is fine. |
| await measure("Modifying JAVA_HOME for JRuby", async () => { | ||
| console.log("attempting to run with existing JAVA_HOME") | ||
|
|
||
| let ret = await exec.exec('ruby', ['--version']) | ||
| let ret = await exec.exec('java', ['-jar', path.join(rubyPrefix, 'lib/jruby.jar'), '--version'], {ignoreReturnCode: true}) |
There was a problem hiding this comment.
This should be $JAVA_HOME/bin/java (or process.env, you get the idea) as java in PATH might resolve to something different and in fact it does I saw /usr/bin/java in logs. Could you make a PR to address that?
Apparently this exec errors when the subprocess fails rather than returning an error code. This causes the whole setup process to terminate. This patch catches the error and uses that to indicate failure to launch.
Hopefully will address the issue in ruby/jruby-dev-builder#10 (comment)