On Tue, 25 Mar 2025 23:58:36 GMT, Volker Simonis <simo...@openjdk.org> wrote:
>> Aleksey Shipilev has updated the pull request with a new target base due to >> a merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains three additional >> commits since the last revision: >> >> - Test cases >> - Merge branch 'master' into JDK-8352533-jspawnhelper-ioexceptions >> - Initial fix >> >> Good spawnhelper failure message >> >> Trying tests >> >> Print helper message on all IOExceptions >> >> Final > > src/java.base/unix/native/libjava/ProcessImpl_md.c line 806: > >> 804: case sizeof(errnum): >> 805: waitpid(resultPid, NULL, 0); >> 806: throwIOException(env, errnum, "Exec failed"); > > Why do we need to keep `throwIOException()` for this single call site? Can't > we replace it with `throwInternalIOException()` as well? > > Otherwise looks good. Because this is the path through which we report "normal" errors like "command not found", "path does not exist", etc. -- that are not related to jspawnhelper failures at all. So it would be dubious to print the warnings then. I added a few test cases in `Basic.java` that checks for `IOException` messages int those cases. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24149#discussion_r2014649477