On Mon, 4 Jul 2022 22:32:20 GMT, Yasumasa Suenaga <ysuen...@openjdk.org> wrote:

>> configure failed as following when I run it on WSL 1.
>> 
>> 
>> checking for version string... 20-internal-adhoc.yasuenag.jdk
>> configure: Found potential Boot JDK using configure arguments
>> configure: The command for java_to_test, which resolves as 
>> "/mnt/d/java/jdk-18/bin/java", can not be found.
>> configure: error: Cannot locate /mnt/d/java/jdk-18/bin/java
>> configure exiting with result code 1
>> 
>> 
>> fixpath.sh would attempt to add ".exe" (e.g. "java" -> "java.exe") if 
>> `wslpath -w" failed (returns non-zero value). However it returns zero even 
>> if the path does not exist in recent WSL (v0.61.4 at least).
>> 
>> WSL v0.60.0.0
>> 
>> $ wslpath -w silver-bullet
>> wslpath: silver-bullet: No such file or directory
>> $ echo $?
>> 1
>> 
>> 
>> WSL v0.61.4.0
>> 
>> $ wslpath -w silver-bullet
>> silver-bullet
>> $ echo $?
>> 0
>> 
>> 
>> We should add ".exe" at the tail of path regardless of return value from 
>> wslpath.
>
> Yasumasa Suenaga has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update the fix to use unixpath

This looks better with less changes. Note that GHA tests msys2 nowadays, not 
Cygwin. Also, how do you exercise the four different test cases per platform I 
requested using GHA?

While this looks better, I'm still not convinced this is correct. What if the 
path is not an executable? Now you unconditionally add .exe, even if the .exe 
does not exist. I think import_path() can be used to import e.g. directories, 
too, right? So if the starting path does not exist, but the .exe version of it 
exists, it seems quite safe and correct to swap it out for the .exe version. 
But I don't think you can just do it unconditionally.

Apart from testing running `fixpath import` with an executable both in the env 
root file namespace, and with the Windows file namespace -- both with and 
without .exe -- I also think you need to test with a directory, in both 
namespaces. (Ideally, we'd have automatic tests for this kind of things, but 
it's been too complicated to write tests that can setup a proper environment, 
so we're forced to do systematic manual testing whenever we change this code.)

-------------

PR: https://git.openjdk.org/jdk/pull/9357

Reply via email to