Fix #101873: wmic not found on insider builds of Windows 11 #101874

Xavier Hallade · 2024-09-17T13:44:21+02:00

Xavier Hallade commented

2024-09-17 13:44:21 +02:00

First-time contributor

use powershell instead

Xavier Hallade added 1 commit 2024-09-17 13:44:22 +02:00

Fix #101873 : wmic not found on insider builds of Windows 11 1d8254acdd

use powershell instead

Xavier Hallade requested review from Sergey Sharybin 2024-09-17 13:44:41 +02:00

Xavier Hallade commented

2024-09-17 13:50:22 +02:00

First-time contributor

Note I've only verified on my machine - I don't have a dual socket Windows system to verify these changes on.

Sergey Sharybin commented

2024-09-17 16:51:40 +02:00

I am not sure if powershell is available o nWindows 8, and whether it is always guaranteed to be called powershell and not pwsh. Maybe Ray have some better insights. I can't tag him here, perhaps because he is not in the infra team.

I am not sure if powershell is available o nWindows 8, and whether it is always guaranteed to be called `powershell` and not `pwsh`. Maybe Ray have some better insights. I can't tag him here, perhaps because he is not in the infra team.

Ray molenkamp commented

2024-09-17 17:05:49 +02:00

First-time contributor

not a frequent powershell user, but according to wikipedia windows 10 and 11 come with powershell 5.1 out of the box (which uses powershell.exe) so that should be ok.

windows 8.x won't have it though, so we may have to do a

if exists in path wmic.exe { do wmic }
else if exists in path powershell.exe { do powershell }
else { throw hands up in the air and give up }

not a frequent powershell user, but according to [wikipedia](https://en.wikipedia.org/wiki/PowerShell#Windows_PowerShell_5.1) windows 10 and 11 come with powershell 5.1 out of the box (which uses powershell.exe) so that should be ok. windows 8.x won't have it though, so we may have to do a ``` if exists in path wmic.exe { do wmic } else if exists in path powershell.exe { do powershell } else { throw hands up in the air and give up } ```

Xavier Hallade added 2 commits 2024-09-18 08:56:19 +02:00

readd wmic for Windows < 10 52ef1b8b51

move windows cores count to cpu_cores module 2db4040060

Xavier Hallade commented

2024-09-18 09:00:09 +02:00

First-time contributor

Ah, I didn't picture Windows 8 was still supported.
Rather than testing if wmic exists, checking on windows version directly sounds cleaner/easier to maintain.
I've added a commit to do this and then.. code looked a bit big in this area, I moved it to cpu_cores and then noticed it was an already existing library. I'm not sure if you want to keep the code separate or extend cpu_cores as I did?

The current build instructions didn't fly on my machine (not finding autogen.sh in freetype when building from msys2/mingw64), I've only tested my changes in python outside of the launcher.

Ah, I didn't picture Windows 8 was still supported. Rather than testing if wmic exists, checking on windows version directly sounds cleaner/easier to maintain. I've added a commit to do this and then.. code looked a bit big in this area, I moved it to cpu_cores and then noticed it was an already existing library. I'm not sure if you want to keep the code separate or extend cpu_cores as I did? The current build instructions didn't fly on my machine (not finding autogen.sh in freetype when building from msys2/mingw64), I've only tested my changes in python outside of the launcher.

Ray molenkamp commented

2024-09-18 13:42:40 +02:00

First-time contributor

Rather than testing if wmic exists, checking on windows version directly sounds cleaner/easier to maintain.

Rather than knowing for sure the thing you're about to execute exists feels cleaner than making the assumption it exists based on on a different heuristic? I'm not sure i agree with that, on the other hand, I expect your way to still work just fine

> Rather than testing if wmic exists, checking on windows version directly sounds cleaner/easier to maintain. Rather than knowing for sure the thing you're about to execute exists feels cleaner than making the assumption it exists based on on a different heuristic? I'm not sure i agree with that, on the other hand, I expect your way to still work just fine

Xavier Hallade commented

2024-09-18 13:55:44 +02:00

First-time contributor

I have been a bit vague on my reasons but I believe I have quite good ones:

wmic being standard prior to Windows 10 is based on facts, it's a safe heuristic, not going to change. Only exception is if windows install is deliberately crippled - already an issue with current implementation.
The day you stop supporting ancient Windows versions, the code under old-Windows condition is easier to remove
This condition privileges a non-deprecated tool
Microsoft could introduce a new wmic.exe executable that tells you wmic is gone or worse (see what happens when running python.exe on a Windows OS that doesn't have python if you don't believe in this being likely at all :|)

I have been a bit vague on my reasons but I believe I have quite good ones: - wmic being standard prior to Windows 10 is based on facts, it's a safe heuristic, not going to change. Only exception is if windows install is deliberately crippled - already an issue with current implementation. - The day you stop supporting ancient Windows versions, the code under old-Windows condition is easier to remove - This condition privileges a non-deprecated tool - Microsoft could introduce a new wmic.exe executable that tells you wmic is gone or worse (see what happens when running python.exe on a Windows OS that doesn't have python if you don't believe in this being likely at all :|)

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions.