Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(windows): add windir and C:/Windows as fallback on non-WSL #328

Merged
merged 1 commit into from Dec 22, 2023

Conversation

pikax
Copy link
Contributor

@pikax pikax commented Dec 22, 2023

related: vitejs/vite#14292 vitejs/vite#14293

Not seeing SYSTEMROOT being provided on the environment on my system.

System:
    OS: Windows 11 10.0.22621
    CPU: (32) x64 AMD Ryzen 9 7950X 16-Core Processor
    Memory: 35.64 GB / 63.14 GB
  Binaries:
    Node: 20.10.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.21 - C:\Program Files\nodejs\yarn.CMD
    npm: 10.2.3 - C:\Program Files\nodejs\npm.CMD
    pnpm: 8.12.0 - ~\AppData\Local\pnpm\pnpm.EXE
  Browsers:
    Edge: Chromium (120.0.2210.77)
    Internet Explorer: 11.0.22621.1

windir seems to be providing the same path and just in case added the default C:\\Windows as a fallback.

@sindresorhus
Copy link
Owner

The problem here is your system as SystemRoot is defined by default by the system, so you must have unset it somehow.

@sindresorhus sindresorhus merged commit 8e69be4 into sindresorhus:main Dec 22, 2023
2 checks passed
@pikax pikax deleted the fix_window_path branch December 22, 2023 11:19
@pikax
Copy link
Contributor Author

pikax commented Dec 22, 2023

The problem here is your system as SystemRoot is defined by default by the system, so you must have unset it somehow.

That explanation is the one that makes more sense, but I don't recall changing/removing it, it could be removed by installing some software I reckon.

Just for FYI, here's my current Environment variables, I believe the only thing I'll change be Path, the rest of the variables are either set by installing software or by system default.

image

Is a very odd behaviour that variable is not present, regardless, this change should probably prevent that issue from happening since there's two fallbacks.

@brc-dd
Copy link

brc-dd commented Dec 22, 2023

Closes #205 and #292. But the core issue is probably with Node.js itself here as Sindre commented on #299.

@pikax Can you try logging process.env.SystemRoot instead of process.env.SYSTEMROOT in node? I don't think systemroot shows there in the settings - it's a readonly variable automatically added by windows.

@pikax
Copy link
Contributor Author

pikax commented Dec 22, 2023

I can confirm SystemRoot is being provided instead of Uppercased

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants