winVersion/Windows 10 version fetch: use Windows Registry instead of a static release names to builds #12544

josephsl · 2021-06-13T07:50:12Z

Hi,

Mostly in preparation for a possibility that Windows 10 Version 21H2 will be used by multiple builds, including Server 2022, and prepares NVDA to support Windows 11 and future Windows versions:

Link to issue number:

Closes #12509

Summary of the issue:

Currently a static Windows 10 release names to builds map is used to record Windows 10 builds to feature updates (releases). Until 21H1 there was a one-to-one correspondence between releases and builds, but it appears Version 21H2 will be represented by multiple builds:

Windows 10: 19044
Server 2022: 20348
Windows 11: build 22000

Builds subject to confirmation.

Therefore this breaks current static map.

Description of how this pull request fixes the issue:

Since Version 1511, Windows Registry holds release name which corresponds to builds (e.g. 1511 = 10586, 20H2 = 19042), housed inside a "Release Id" key. Starting with Version 20H2, a separate "DisplayVersion" key is used to announce public release name. In Windows Insider Preview builds, dev channel has display version of "Dev", making it even easier to determine what kind of build a user is running. This tradition continues with Windows 11 and Server 2022, with Registry recording "21H2", which makes sense given their projected release period. Therefore use Windows Registry instead of the static releases to builds map and deprecate releases to builds map.

Testing strategy:

Manual testing:

On supported Windows 10 releases (1507 LTS, 1607/server 2016 LTS, 1809/Server 2019 LTS, 1909 (Enterprise and Education), 2004, 20H2, 21H1, 21H2/Server 2022 preview, Windows Insider Preview dev channel), open Windows Registry path described in Windows 10 versions: use Windows Registry to obtain release Id/display name #12509 and confirm that release Id's and display name keys are present (the latter is available starting with 20H2).
In Python Console, verify that winVersion.getWinVer() says "Windows 10 YYMM/YYHn" and the last part (release name) matches the value obtained from Windows Registry.
Optionally, define a local unit test that checks the version string returned by winVersion.getWinVer() starts with "Windows 10 release" for the local system (e.g. "Windows 10 21H1" on build 19043).

Known issues with pull request:

None

Change log entries:

Likely a change for developers, stating that on Windows 10, release names are obtained from Windows Registry, although note that this is an internal operation. A visible changelog is:

Changes for developers:

On Windows 10 Version 1511 and later (including Insider Preview builds), current Windows feature update release name is obtained from Windows Registry. (#12509)
The releases to builds mapping found in winVersion module is deprecated and will be removed in 2022.1. (#12544)

Code Review Checklist:

Pull Request description is up to date.
Unit tests.
System (end to end) tests.
Manual testing.
User Documentation.
Change log entry.
Context sensitive help for GUI changes.
UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers

Discussion:

If this PR is adopted, what should we do about the static release names to builds map? There are three possibilities:

Swap releases with builds so the map records build : release. This makes Windows Registry method unnecessary - adding future Windows releases will continue to involve editing this map.
Deprecate and remove the static map - the only place the map is used is when fetching Windows 10 release names anyway, and since Windows Registry contains needed information, there is no need for this map. If deprecation and removal are chosen, a follow-up PR should be created during 2022.1 development cycle to remove the static map altogether.
For releases represented by multiple builds, have Windows 10 release name fetcher check for these builds and return the release name for these. This makes version fetch routine complicated as one needs to hunt for all build numbers represented by a single release name.

Update (2021-07-07): deprecated and replaced by a private builds to releases mapping.

Thanks.

josephsl · 2021-06-13T07:52:43Z

Hi,

By the way, people watching NVDA add-ons may notice a similarity with Resource Monitor - in fact, this PR is a simplified version of Windows 10 version fetch routine found in that add-on, therefore it went through years of real-life usage.

Thanks.

cary-rowen · 2021-06-13T08:08:09Z

@josephsl
There is a digression, Windows 10 application enhancement is so versatile; after years of extensive use and testing, why not integrate it into NVDA itself?
Thanks.

josephsl · 2021-06-13T08:18:26Z

Hi, not all features from that add-on can be integrated into NVDA because Windows 10 apps (built-in and from Microsoft Store) can change without notice, which also applies to features found in Windows Insider Preview builds (things come and go without notice, making it harder to say something is stable enough to be part of NVDA unless it’s been out there for a while). To be honest, Windows 10 App Essentials is “versatile” because of the state of app and feature accessibility and usability across Windows 10 releases (many commits in this add-on are frustrations about app accessibility and usability issues, which have lessened somewhat in recent years). Another key consideration is support duration for Windows 10 releases – some features are part of Windows 10 releases no longer supported by Microsoft, therefore if support for them ends in the add-on, I promptly remove the code associated with it (the add-on is currently going through such a transition to fully remove support for releases earlier than Version 2004). Thanks.

cary-rowen · 2021-06-13T10:39:00Z

Thanks @josephsl for reply

leonardder

I assume it will suffice to remove the release map altogether.

source/winVersion.py

josephsl · 2021-06-14T07:24:11Z

Hi, I once saw Registry access used as a context manager but wasn’t sure if it is applicable; now you mention it, I’ll perform some tests and make necessary changes (this is also applicable to Resource Monitor add-on, come to think of it). Thanks for this tip.

josephsl · 2021-06-15T05:36:04Z

Hi,

In the end, there is no point in keeping something that will soon break (look at various 21H2 build branches), so the releases map will be gone.

Thanks.

feerrenrut

I'm ok with getting the Release name like this (from the registry) when the getWinVer method is called. But for the constants it would be better to stick to an in code mapping, perhaps instead of the WIN10_RELEASE_NAME_TO_BUILDS dictionary, add an argument to class WinVersion to set the release name and set all the names with the constants. Then there can be a constant for each variation of "21H2 " EG something like WIN10_21H2_SERVER_2022 = WinVersion(major=10, minor=0, build=20348, releaseName="Windows 10 21H2 Server 2022")

source/winVersion.py

josephsl · 2021-06-22T08:48:46Z

I'm ok with getting the Release name like this (from the registry) when the getWinVer method is called. But for the constants it would be better to stick to an in code mapping, perhaps instead of the WIN10_RELEASE_NAME_TO_BUILDS dictionary, add an argument to class WinVersion to set the release name and set all the names with the constants. Then there can be a constant for each variation of "21H2 " EG something like WIN10_21H2_SERVER_2022 = WinVersion(major=10, minor=0, build=20348, releaseName="Windows 10 21H2 Server 2022")

Hi,

Adding "releaseName": this will also benefit Windows 7 and 8.x.

Windows Registry and release name: yes, Windows Registry will return the correct release name depending on the version you've got, so it can be used in winVersion.getWinVer function, I was wondering about unit tests - I'll come up with soething.

Release names to builds map: not needed as Windows Registry and release name argument will suffice.

Caching release name from the module: right, that is a useful optimization strategy.

Would you like me to restore the release names to builds dictionary for 2021.x and mark it as deprecated in the process?

Thanks.

josephsl · 2021-06-22T11:16:38Z

Hi,

For Windows Server 2022, at least "Windows 10 21H2" will be returned, which is fine. The overall PR could change depending on what we learn on June 24th.

Thanks.

josephsl · 2021-06-22T11:19:07Z

Hi,

As for unit test, str.startswith should be checked as repr records release name, build, and product type in one go.

Thanks.

source/winVersion.py

…ccess#12509. From Widows 10 Version 1511 (November Update/build 10586), WinVer (About Windows) dialog uses Windows Registry to obtain version information (build 10240 wsa retroactively named '1507' then). Initially 'ReleaseId' key was used, but since 20H2, 'DisplayVersion' is used (ReleaseId for 20H2 is 2009). The Registry method is needed to support multiple Windows 10 builds with same display version. Currently release Id's to builds map contains one to one mapping between versions and builds, whereas in 21H2 development cycle, up to three builds may represent this release. Therefore use Registry method to detect all public Windows 10 releases.

…ager. re nvaccess#12509. Advised by Leonard de Ruijter: use context manager (with) associated with Windows Registry HKEY handles (of the form 'with winreg.openKey(HKEY, path) as something').

… Re nvaccess#12509. Windows 10 releases to builds map records feature update release names to builds (e.g. '1903': 18362). With the Windows Registry method in place, coupled with several build branches for Version 21H2, it makes sense to remove an unused dictionary.

…ess#12509. Advice from Reef Turner (NV Access): define a 'release name' atribute to set the public facing name for a Windows release such as 'Windows 7' for 6.1.7600. This is useful for Widows 10 due to many major updates released since July 2015.

…#12509. If release name is defined, return this as the version name e.g. if 'releaseName' is 'Windows 10 1511' for build 10586, return 'Windows 10 1511' instead of using release names to builds map and/or Windows Registry. Note that the implied 'else' block is not indented.

…ess#12509. Although release name texts are useful for Windows 10, apply this change to all constants (Windows 7 and 8.x also).

Note that on server systems, unless otherwise noted, client relesae names are returned. The first such exception is likely the erver counterpart of Windows 10 21H2 (Server 2022) which may have a different build than client releases.

Review by Reef Turner (NV Access): in winVersion.getWinVer() function, instead of opening Windows Registry to obtain Windows 10 each time the function is called, have the version string handy from the beginning. Note that Release Id and display version are not found on Windows 10 1507 (build 1024) or earlier.

Advised by Reef Turner (NV Access): define a unit test to compare release names, specifically to simulate winVersion.getWinVer but with a constant. Because repr(winVersion.getWinVer()) includes release name, major.minor.build, and product type in one go, use asertIn to test the expected string (in this case, 'Windows 10 1607' for Server 2016).

…lds map. Re nvaccess#12509." This reverts commit e5f39aa.

…ved in 2022.1. Re nvaccess#12509. Advised by NV Access: mark no longer used releases to builds map as deprecated and to be removed in 2022.1 as Windows 10 release names are obtained from Windows Registry.

… to release names' map. Re nvaccess#12509. Note from Reef Turner (NV access): flip the now deprecated 'release names to builds' map by defining a new map that records builds to releases. This prepares NVDA to support Windows releases with multiple builds under the same release name, as well as defining the release name for build 10240 (Windows 10 1507) through this mapping instead of hard-coding it in functions.

…e determined. re nvaccess#12509. On Windows 10 (and later), the following pths will be used to retrieve relase name (in this order): 1. Existing build: whatever the release name is. 2. Current version: Windows Registry. This also covers Windows Insider Preview builds that will eventually become stable (retail) builds. If none of these work, just say 'Windows 10 unknown'. This also resolves Flake8 F821 (undefined variable name).

.

AppVeyorBot · 2021-06-28T11:43:32Z

PASS: Translation comments check.
FAIL: Unit tests. See test results for more information.
PASS: Lint check.
PASS: System tests.
Build (for testing PR)

See test results for failed build of commit 28d88b133e

…ting on public uilds. Re nvaccess#12509. Windows Insider Preview builds will say 'Windows 10 Dev', but as unit tests are tested in public builds (unless someone tests in sindier builds), test to see if 'unknown' is returned. Checkig this string is fine because build 21390 is an Insider Preview build that were succeeded by newer builds.

lukaszgo1 · 2021-06-28T11:57:08Z

source/winVersion.py

+
+@functools.lru_cache(maxsize=1)
+def _getRunningVersionNameFromWinReg() -> str:
+	"""Returns the Windows release name defined in Widnows Registry.


Widnows -> Windows

josephsl · 2021-06-28T12:13:32Z

Hi, you’re right – thanks for catching this.

lukaszgo1 · 2021-07-02T09:47:52Z

source/winVersion.py

@@ -26,20 +72,26 @@ def __init__(
 			major: int = 0,
 			minor: int = 0,
 			build: int = 0,
+			releaseName: Optional[str] = "",


This is quite unusual. The standard usage for Optional is for arguments which are set to None by default yet can accept a given type. Here however you're setting it into an empty string as a default value and passing None when constructing the instance. Can this be simplified to either always using empty string and not accepting None or preferably using None as default value for this kwarg?

source/winVersion.py

josephsl · 2021-07-02T09:52:54Z

Hi, not really as this map is only used to look up Windows 10 releases, and no-one uses it. I feel it should have been a private map to begin with, but…

…ccess#12509

feerrenrut

Code looks good now.

Let's add support for Windows 11 in a follow up PR.
Can you please update the PR description, including deprecations in the section for the changes file.

josephsl · 2021-07-07T12:43:22Z

Hi,

PR intro comment updated with what's new entries (any other changes needed?).

Thanks.

feerrenrut · 2021-07-08T04:05:42Z

Yes, looks good @josephsl.

feerrenrut

I think it is more useful if the deprecation entry in the change log points to the PR rather than the issue.

josephsl · 2021-07-08T04:24:10Z

Hi, agreed. Thanks.

Opened against beta since this PR fixes regression which should be fixed before 2021.2 Link to issue number: Fixes #12666 Fix-up of #12544 Summary of the issue: In some rare cases winVersion.getWinVer can cause recursion errors since it calls winVersion._getRunningVersionNameFromWinReg which in turn executes winVersion.getWinVer to check if the currently running version of Windows contains release info in the registry. Description of how this pull request fixes the issue: Rather than checking if the current version of Windows contains release info in the registry by comparing it to the lowest version for which this is supported I'm just checking if the necessary info exists in the registry and if not appropriate exceptions are raised. To avoid pointless registry accesses result of getWinVer is cached - version of Windows on which NVDA is running is not going to change during its lifetime. * Fix rare recursion error in the winVersion module * Avoid pointless registry accesses by caching results of `getWinVer` as the version of Windows on which we're running cannot be changed.

Removes deprecated code from #12509 Summary of the issue: As of #12544, Windows 10 releases to builds map was deprecated, with what's new document declaring that this map will be gone in 2022.1. This pull request accomplishes exactly that. Description of how this pull request fixes the issue: Removes deprecated Windows 10 releases to builds as Windows Registry is used to obtain Windows 10/11 release information.

josephsl requested a review from a team as a code owner June 13, 2021 07:50

josephsl requested a review from michaelDCurran June 13, 2021 07:50

leonardder suggested changes Jun 14, 2021

View reviewed changes

source/winVersion.py Outdated Show resolved Hide resolved

feerrenrut suggested changes Jun 22, 2021

View reviewed changes

source/winVersion.py Outdated Show resolved Hide resolved

source/winVersion.py Show resolved Hide resolved

source/winVersion.py Show resolved Hide resolved

feerrenrut reviewed Jun 23, 2021

View reviewed changes

source/winVersion.py Outdated Show resolved Hide resolved

feerrenrut reviewed Jun 23, 2021

View reviewed changes

source/winVersion.py Outdated Show resolved Hide resolved

josephsl added 13 commits June 23, 2021 10:53

winVersion: releaseID -> releaseId, clarify comment. Re nvaccess#12509

a91bd41

winVersion/Windows 10 version fetch: use Windows Registry context man…

b99b80e

…ager. re nvaccess#12509. Advised by Leonard de Ruijter: use context manager (with) associated with Windows Registry HKEY handles (of the form 'with winreg.openKey(HKEY, path) as something').

WinVersion constants: define releae names for all constants. Re nvacc…

7a4343f

…ess#12509. Although release name texts are useful for Windows 10, apply this change to all constants (Windows 7 and 8.x also).

WinVersion: clarify comment. Re nvaccess#12509.

475d1c2

Note that on server systems, unless otherwise noted, client relesae names are returned. The first such exception is likely the erver counterpart of Windows 10 21H2 (Server 2022) which may have a different build than client releases.

WinVersion: actual name for 'releaseName' flag. Re nvaccess#12509

a185857

Revert "winVersion/Windows 10 releases: remove unused releases to bui…

f391a2d

…lds map. Re nvaccess#12509." This reverts commit e5f39aa.

winVersion/releases to builds map: marked for deprecation, to be remo…

1a199b4

…ved in 2022.1. Re nvaccess#12509. Advised by NV Access: mark no longer used releases to builds map as deprecated and to be removed in 2022.1 as Windows 10 release names are obtained from Windows Registry.

josephsl force-pushed the win10VersionFromWinReg branch from e7c0002 to 1a199b4 Compare June 23, 2021 17:58

josephsl added 3 commits June 28, 2021 04:22

winVersion: lint, make releaseName parameter optional. Re nvaccess#12509

3c38a44

.

Merge remote-tracking branch 'origin/master' into win10VersionFromWinReg

39f0812

lukaszgo1 reviewed Jun 28, 2021

View reviewed changes

josephsl added 2 commits June 28, 2021 05:22

winVersion/WinReg: typo

7d71d6b

Merge branch 'master' into win10VersionFromWinReg

721818b

lukaszgo1 reviewed Jul 2, 2021

View reviewed changes

source/winVersion.py Show resolved Hide resolved

josephsl added 2 commits July 2, 2021 02:53

Merge branch 'master' into win10VersionFromWinReg

6f25e0d

winVersion.WinVersion/release name: make it 'None' by default. Re nva…

0e010d2

…ccess#12509

feerrenrut suggested changes Jul 7, 2021

View reviewed changes

feerrenrut approved these changes Jul 8, 2021

View reviewed changes

update changes file for PR nvaccess#12544

8fe2852

feerrenrut approved these changes Jul 8, 2021

View reviewed changes

feerrenrut merged commit 8774916 into nvaccess:master Jul 8, 2021

nvaccessAuto added this to the 2021.2 milestone Jul 8, 2021

josephsl mentioned this pull request Jul 8, 2021

winVersion: build 22000 is not recognized as Windows 11 #12626

Closed

josephsl deleted the win10VersionFromWinReg branch July 8, 2021 21:43

lukaszgo1 mentioned this pull request Jul 28, 2021

Fix rare recursion error in the winVersion module #12696

Merged

8 tasks

josephsl mentioned this pull request Jan 3, 2022

winVersion: remove deprecated Windows 10 releases to builds map #13211

Merged

5 tasks

xuvatilavv mentioned this pull request Feb 19, 2022

Fix reported version of newer Windows 10 releases CleverRaven/Cataclysm-DDA#55506

Merged

winVersion/Windows 10 version fetch: use Windows Registry instead of a static release names to builds #12544

winVersion/Windows 10 version fetch: use Windows Registry instead of a static release names to builds #12544

josephsl commented Jun 13, 2021 •

edited

josephsl commented Jun 13, 2021

cary-rowen commented Jun 13, 2021

josephsl commented Jun 13, 2021 via email

cary-rowen commented Jun 13, 2021

leonardder left a comment

josephsl commented Jun 14, 2021 via email

josephsl commented Jun 15, 2021

feerrenrut left a comment

josephsl commented Jun 22, 2021

josephsl commented Jun 22, 2021

josephsl commented Jun 22, 2021

AppVeyorBot commented Jun 28, 2021

lukaszgo1 Jun 28, 2021

josephsl commented Jun 28, 2021 via email

lukaszgo1 Jul 2, 2021

josephsl commented Jul 2, 2021 via email

feerrenrut left a comment

josephsl commented Jul 7, 2021

feerrenrut commented Jul 8, 2021

feerrenrut left a comment •

edited

josephsl commented Jul 8, 2021 via email

winVersion/Windows 10 version fetch: use Windows Registry instead of a static release names to builds #12544

winVersion/Windows 10 version fetch: use Windows Registry instead of a static release names to builds #12544

Conversation

josephsl commented Jun 13, 2021 • edited

Link to issue number:

Summary of the issue:

Description of how this pull request fixes the issue:

Testing strategy:

Known issues with pull request:

Change log entries:

Code Review Checklist:

Discussion:

josephsl commented Jun 13, 2021

cary-rowen commented Jun 13, 2021

josephsl commented Jun 13, 2021 via email

cary-rowen commented Jun 13, 2021

leonardder left a comment

Choose a reason for hiding this comment

josephsl commented Jun 14, 2021 via email

josephsl commented Jun 15, 2021

feerrenrut left a comment

Choose a reason for hiding this comment

josephsl commented Jun 22, 2021

josephsl commented Jun 22, 2021

josephsl commented Jun 22, 2021

AppVeyorBot commented Jun 28, 2021

lukaszgo1 Jun 28, 2021

Choose a reason for hiding this comment

josephsl commented Jun 28, 2021 via email

lukaszgo1 Jul 2, 2021

Choose a reason for hiding this comment

josephsl commented Jul 2, 2021 via email

feerrenrut left a comment

Choose a reason for hiding this comment

josephsl commented Jul 7, 2021

feerrenrut commented Jul 8, 2021

feerrenrut left a comment • edited

Choose a reason for hiding this comment

josephsl commented Jul 8, 2021 via email

josephsl commented Jun 13, 2021 •

edited

feerrenrut left a comment •

edited