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.

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

Handle errors whe looking for Java executable #8648

Merged
merged 1 commit into from
Feb 22, 2025

Conversation

mrnoname1000
Copy link
Contributor

Should fix jruby/jruby-launcher#44. Doesn't explicitly check if java is a directory but command -v will fail because it's not a file that can be executed.

@headius
Copy link
Member

headius commented Feb 21, 2025

This should be rebased against master.

If I'm reading right, these are launching a subshell to check the java executable, yes? Can't we just check if it is a normal executable file more cheaply?

@enebo
Copy link
Member

enebo commented Feb 21, 2025

If java must get launched I think we entertain java -Xinternalversion, This would not only test validity but could also be used for proper version handling. This is only 0.004s on my machine (I guess MacOS is higher).

@mrnoname1000
Copy link
Contributor Author

If I'm reading right, these are launching a subshell to check the java executable, yes?

It is launching a subshell but it was always doing that because it's necessary to capture the output of command -v (which is a pure-shell alternative to which). It's not actually running any external programs, it's just checking that there's a file called java in $PATH that's executable. We could check that it succeeds before running it in a subshell but I don't think it's really necessary to save one fork on a code path that's going to error anyway.

@headius
Copy link
Member

headius commented Feb 21, 2025

@mrnoname1000 Ok that seems find then. We can merge this once you rebase it on master.

@mrnoname1000 mrnoname1000 force-pushed the launcher-javacmd-errors branch from 563c613 to 59855f3 Compare February 21, 2025 23:06
@mrnoname1000 mrnoname1000 changed the base branch from 10-dev to master February 21, 2025 23:14
@headius
Copy link
Member

headius commented Feb 21, 2025

We can merge this, as it is a good change, but it doesn't actually fix jruby/jruby-launcher#44. That issue is actually about the native launcher, not the shell script.

@headius headius merged commit f6306c2 into jruby:master Feb 22, 2025
95 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Launcher fails with "execv failed: Permission denied (13)"
3 participants