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

Reply via email to