On Tue, 31 Mar 2026 22:27:00 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 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:/.
make/autoconf/basic_tools.m4 line 73:
> 71:
> 72: SYSTEM32_PATH=$($PATHTOOL -u "${SYSTEMROOT}/system32" 2>/dev/null)
> 73: UTIL_LOOKUP_PROGS(CMD, cmd.exe, $PATH:${SYSTEM32_PATH})
The problem here is that `cmd.exe` is being setup as a "fundamental" tool,
before we probe for the platform, because it's needed in
`BASIC_SETUP_PATHS_WINDOWS`. So to be able to look it up this early, we make
some assumptions on the PATH, which clearly aren't correct on all systems.
OTOH, the current logic should work if you have `$SYSTEMROOT/system32` on your
PATH when invoking configure. The hard coded `/cygdrive/c/windows/system32` is
only meant as a fallback as far as I understand. Is there a reason that you
don't have this in your PATH?
I don't think we should fill up this macro with more hacks. If this really
needs to be fixed, we should probably add something like
`BASIC_SETUP_FUNDAMENTAL_TOOLS_WINDOWS` that gets called after the platform
setup, but before path setup, where `CMD` can be defined in a context where
it's known that we are on Windows.
make/autoconf/toolchain_microsoft.m4 line 341:
> 339: # Make sure we only capture additions to PATH needed by VS.
> 340: # Clear out path, but need system dir present for vsvars cmd file to
> be able to run
> 341: export PATH=$(${PATHTOOL} -u "${SYSTEMROOT}")/system32
I think this is ok.
make/scripts/fixpath.sh line 77:
> 75: drive_letter_lower="$(echo "${drive_letter}" | tr 'A-Z' 'a-z')"
> 76: cyg_systemroot="$(${PATHTOOL} -u "${SYSTEMROOT}")"
> 77: DRIVEPREFIX="${cyg_systemroot%%/${drive_letter_lower}/*}"
We should not use "cyg" as prefix for variables here unless they are truly
cygwin specific. I believe the term we use for the cyg/wsl/msys style paths is
"env".
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30523#discussion_r3028525840
PR Review Comment: https://git.openjdk.org/jdk/pull/30523#discussion_r3028535192
PR Review Comment: https://git.openjdk.org/jdk/pull/30523#discussion_r3028713943
