On Thu, 2 Apr 2026 01:00:03 GMT, Yasumasa Suenaga <[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 to use the `SYSTEMROOT`
>> environment variable, which points to the Windows installation path,
>> instead of hardcoded references to C:/Windows.  This patch also updates
>> tests to not use the presence of C:/ to detect Windows (instead relying
>> on the output of `uname -s`) and to not assume that every Windows
>> installation has a C:/.
>
> After your change, the linker reports following arguments:
> 
> /out:conftest.exe
> -libpath:c:\progra~1\micros~1\18\community\vc\tools\msvc\14.50.35717\atlmfc\lib\x64
> -libpath:c:\progra~1\micros~1\18\community\vc\tools\msvc\14.50.35717\lib\x64
> -libpath:.
> -libpath:.
> -libpath:.
> conftest.obj
> 
> 
> HEAD of master works as following. I guess it implies path resolution is 
> wrong on WSL.
> 
> /out:conftest.exe
> -libpath:c:\progra~1\micros~1\18\community\vc\tools\msvc\14.50.35717\atlmfc\lib\x64
> -libpath:c:\progra~1\micros~1\18\community\vc\tools\msvc\14.50.35717\lib\x64
> -libpath:c:\progra~2\wi3cf2~1\netfxsdk\4.8\lib\um\x64
> -libpath:c:\progra~2\wi3cf2~1\10\lib\100261~1.0\ucrt\x64
> -libpath:c:\progra~2\wi3cf2~1\10\lib\100261~1.0\um\x64
> conftest.obj

@YaSuenag  Thanks for letting me know about the WSL issue and for diving into 
the details.  And thanks also to @erikj79 for pointing out that not all 
references to C: occur with the intent of reading/writing files or directories. 
 Indeed, the zeal to get rid of all hardcoded references wasn't helpful. :)

I've added a commit that gets rid of the `SYSTEMROOT` environment variable, 
whose absence on WSL2 was the root of the issue.  Here's the full commit 
message that explains the details.

> 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.

I'll update the PR description to reflect the change.

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

PR Comment: https://git.openjdk.org/jdk/pull/30523#issuecomment-4180776459

Reply via email to