On Thu, 2 Apr 2026 22:15:08 GMT, Ashay Rane <[email protected]> wrote:

>> Prior to this patch, both the build and the tests included several
>> references to C:/.  References in the build assumed that Windows is
>> installed on C:/, which causes the build to fail when Windows is
>> installed on a different drive.  The references among tests assumed that
>> the C:/ drive exists, which although mostly correct, is not guaranteed,
>> making the tests fragile.
>> 
>> This patch fixes the build references using the path derived from that
>> to cmd.exe, which itself is discovered from the PATH environment
>> variable.  We deliberately avoid using `SYSTEMROOT` and related
>> variables since these are not set on WSL2.  This patch also updates the
>> test to not use the presence of C:/ to detect Windows.  Instead, the
>> test now probes the `java -XshowSettings:all` output to know whether the
>> test is running on Windows.
>
> Ashay Rane has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix Windows build so that it works on both Cygwin and on WSL
>   
>   The key reason that the previous patch broke the build on WSL but not
>   Cygwin and MSys2 was that on WSL, `SYSTEMROOT` (and related environment
>   variables like `SYSTEMDRIVE`) are not set, whereas the previous patch
>   was heavily reliant on these environment variables to find the drive
>   letter.  With `SYSTEMROOT` absent on WSL, fixpath.sh received invalid
>   paths as arguments, causing it to output invalid paths like ":", thus
>   ultimately breaking the build in strange ways.
>   
>   As Erik Joelsson pointed out, the build discovers the path to cmd.exe
>   early in the process using the `PATH` env var alone (Cygwin, MSys2, and
>   WSL all inherit the full Windows PATH).  With the location of cmd.exe
>   reliably discovered no matter the environment, we can now infer the
>   drive letter and other Windows paths from the path to cmd.exe instead of
>   using environment-specific variables.
>   
>   This patch effectively rewrites the previous commit using the path to
>   cmd.exe alone.  Of course, if we aren't able to detect the path to
>   cmd.exe, everything in this patch will fail, but in that case, the build
>   fails early, clearly indicating that there are bigger problems:
>   
>   ```
>   configure: error: Incorrect Windows/wsl2 setup. Could not locate cmd.exe
>   configure exiting with result code 1
>   ```
>   
>   Thanks to Yasumasa Suenaga for discovering the problem.  Validated that
>   this build works on Cygwin, WSL2, Linux, and macOS.

This looks much better, thanks!

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

Marked as reviewed by erikj (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/30523#pullrequestreview-4055700308

Reply via email to