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

wmic is starting to disappear from Windows builds, we can't rely on it anymore.
On Windows 10 and above, we can use powershell instead.
Rather than keeping the cpu_cores Windows implementation separate from the cpu_cores library, as it got bigger, I've moved it inside.
The upstream cpu_cores project is dormant with a last commit 11 years ago and never accepted a PR, so I've added Windows support in-place.

wmic is starting to disappear from Windows builds, we can't rely on it anymore. On Windows 10 and above, we can use powershell instead. Rather than keeping the cpu_cores Windows implementation separate from the cpu_cores library, as it got bigger, I've moved it inside. The upstream cpu_cores project is dormant with a last commit 11 years ago and never accepted a PR, so I've added Windows support in-place.

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

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

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

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 :|)

Sergey Sharybin commented

2024-09-27 17:51:57 +02:00

The advantage of the solution from Ray is that it minimizes possible side-effects on Windows 10, and Windows 11 prior to this PR. As Xavier said, the solution was not verified on a dual-socket system. So from keeping the change safe, with only potential issues on the newer system, following Ray's advice is much more welcome.

It is also weird to modify vendorized package. Unless you plan to upstream it?

The advantage of the solution from Ray is that it minimizes possible side-effects on Windows 10, and Windows 11 prior to this PR. As Xavier said, the solution was not verified on a dual-socket system. So from keeping the change safe, with only potential issues on the newer system, following Ray's advice is much more welcome. It is also weird to modify vendorized package. Unless you plan to upstream it?

Xavier Hallade commented

2024-09-27 18:01:23 +02:00

It'd be best if this solution could be verified on a dual-socket system before being adopted: do you know someone with such system, running Windows ?
If not, sure, Ray's solution is better (still crossing fingers a useless wmic.exe will not appear).

It is also weird to modify vendorized package. Unless you plan to upstream it?

It was equally weird to me to implement part of the feature in-place for one OS and not for the others. The upstream project is dormant with a last commit 11 years ago and never accepted a PR. If you prefer the earlier solution, we can just revert last commit.

It'd be best if this solution could be verified on a dual-socket system before being adopted: do you know someone with such system, running Windows ? If not, sure, Ray's solution is better (still crossing fingers a useless wmic.exe will not appear). > It is also weird to modify vendorized package. Unless you plan to upstream it? It was equally weird to me to implement part of the feature in-place for one OS and not for the others. The upstream project is dormant with a last commit 11 years ago and never accepted a PR. If you prefer the earlier solution, we can just revert last commit.

Ray molenkamp commented

2024-09-27 18:15:34 +02:00

First-time contributor

This whole thing is not a hill to die on for me, either way is fine, the argument about a dummy wmic eventually popping up is a valid concern, so lets just go with what you have now, and if people do end up running into issues we can always improve the heuristics later.

Sergey Sharybin commented

2024-09-30 11:48:27 +02:00

It'd be best if this solution could be verified on a dual-socket system before being adopted

Indeed. If it is verified that'd add confidence to the change.

Do you know someone with such system, running Windows?

Unfortunately, no. All the dual-socket systems here run Linux or BSD.
Will ask maybe Lukas Stockner happened to have access to such machine. Or, otherwise, could try doing a quick Windows install on some EOL machine from a storage room.

It was equally weird to me to implement part of the feature in-place for one OS and not for the others. The upstream project is dormant with a last commit 11 years ago and never accepted a PR.

That is a good point.

Would be good to have explanation somewhere (either commit message, or PR itself). Not sure if you'd prefer the change landing as individual commits or as a squashed commit?

> It'd be best if this solution could be verified on a dual-socket system before being adopted Indeed. If it is verified that'd add confidence to the change. > Do you know someone with such system, running Windows? Unfortunately, no. All the dual-socket systems here run Linux or BSD. Will ask maybe Lukas Stockner happened to have access to such machine. Or, otherwise, could try doing a quick Windows install on some EOL machine from a storage room. > It was equally weird to me to implement part of the feature in-place for one OS and not for the others. The upstream project is dormant with a last commit 11 years ago and never accepted a PR. That is a good point. Would be good to have explanation somewhere (either commit message, or PR itself). Not sure if you'd prefer the change landing as individual commits or as a squashed commit?

Xavier Hallade commented

2024-09-30 12:58:25 +02:00

Today I've been able to find a dual socket machine running Windows to validate my changes, they do work as intended.

Would be good to have explanation somewhere (either commit message, or PR itself). Not sure if you'd prefer the change landing as individual commits or as a squashed commit?

I'm fine either way. We can go with the quickest, which is to edit the description to summarize the explanations, and land as a squashed commit.

Today I've been able to find a dual socket machine running Windows to validate my changes, they do work as intended. > Would be good to have explanation somewhere (either commit message, or PR itself). Not sure if you'd prefer the change landing as individual commits or as a squashed commit? I'm fine either way. We can go with the quickest, which is to edit the description to summarize the explanations, and land as a squashed commit.

👍 1

Sergey Sharybin approved these changes 2024-09-30 17:14:57 +02:00

Sergey Sharybin commented

2024-09-30 17:15:44 +02:00

Thanks for tests. We'd need to actually package the script and update the launcher etc, but that's on us and not something that could happen as a PR.

CC @fsiddi to put on the radar.

Thanks for tests. We'd need to actually package the script and update the launcher etc, but that's on us and not something that could happen as a PR. CC @fsiddi to put on the radar.

Sergey Sharybin merged commit 3ccf13943b into main

2024-09-30 17:26:53 +02:00

Sergey Sharybin referenced this issue from a commit

2024-09-30 17:26:54 +02:00

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

Francesco Siddi commented

2024-10-02 16:22:04 +02:00

The new version of the script is now in production, and will run when selecting Blender 4.2.0 (and future versions). Thank you for your contribution! 🎉

Sign in to join this conversation.

No reviewers