nvda icon indicating copy to clipboard operation
nvda copied to clipboard

Windows versions: recognize build 22621 as Windows 11 22H2

Open josephsl opened this issue 2 years ago • 13 comments

Originally marked as a draft until Windows 10 Version 22H2 (build 19045) is released to Windows Insiders but deferred.

Link to issue number:

Closes parts of #13845

Summary of the issue:

Recognize Windows 22H2 releases. In part 1 (this pull request), recognize 10.0.22621 as Windows 11 Version 22H2 (Nickel/Sun Valley 2).

Description of user facing changes

No user facing changes

Description of development approach

Verified that the following builds will be tied to upcoming Windows feature updates:

  • 19045: Windows 10 22H2 (see additional context as this is out of scope for this PR)
  • 22621: Windows 11 22H2

Build 22621 is available to release preview Windows Insiders, build 19045 forthcoming.

Testing strategy:

Manual testing (idelaly on virtual machines):

  1. Install Windows 11 22H2 (releae preview).
  2. With NVDA running, open Python Console (Control+NVDA+Z) and type "import winVersion; winVersion.getWinVer()" without quotes. Verify that version returned is "Windows 11 22H2".
  3. Verify that the version returned also matches winVersion.WIN11_22H2 object and that it is newer (higher build number) than winVersion.WIN11.
  4. Repeat steps 1 through 3 when Windows 10 22H2 is released to Windows Insiders but compare the version returned with winVersion.WIN10_22H2 and make sure winVersion.WIN10_21H2 < winVersion.WIN10_22H2 < winVersion.WINSERVER_2022 (deferred to a later PR).

Known issues with pull request:

Although it is highly unlikely, Windows 10 22H2's build number could change in the future. The sure sign is which build will be released to Windows Insiders, and indication is that it will be 19045.

Change log entries:

None

Code Review Checklist:

  • [x] Pull Request description:
    • description is up to date
    • change log entries
  • [x] Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • [x] API is compatible with existing add-ons.
  • [x] Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • [x] UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English

Additional context:

Originally this PR added winVersion entry for Windows 10 Version 22H2 but is deferred to a later PR.

josephsl avatar Jul 03 '22 16:07 josephsl

Hi, I expect that will be the case this fall. I can revert the readme commit just before releasing this PR from draft if NV Access agrees with this comment. Thanks.

josephsl avatar Jul 03 '22 17:07 josephsl

Hi,

A slight complication arose today: build 22622 is now flighting to beta channel Insiders, identical to 22621 apart from features being enabled. I plan not to add 22622 unless Microsoft releases a feature update with this build number in the future.

Thanks.

josephsl avatar Jul 05 '22 21:07 josephsl

Hi,

If I'm reading Appveyor installed software list correctly, Windows 11 SDK 10.0.22621 can be used in Appveyor, so no revert necessary. The only thing remaining is release of Windows 10 build 19045 to release preview Insiders, and once that happens, I'll remove the draft flag unless @CodeOfDusk says detecting SV2 (build 22621/nickel) is needed for enhanced Windows Console support.

Thanks.

josephsl avatar Jul 18 '22 18:07 josephsl

Hi,

It appears build 22621 SDK is available in Visual Studio 2022 appveyor image. Also, even with signs poiting to Windows 10 Version 22H2 coming to release preview Insiders in August (at the earliest), I think it would be best to at least let NVDA work with Nickel so Bill and others can work on proper SV2 support. Therefore, I will:

  • Revert 10.0.19045 recognition
  • Revert Windows 11 SDK version requirement change
  • Edit SConstruct to say Windows 11 SDK instead of Windows 10 (and also correct the build number as well)

And then will remove draft PR flag so it can be ready for 2022.4.

Thanks.

josephsl avatar Jul 28 '22 01:07 josephsl

Hi,

Follow-up: as for 10.0.19045 recognition, I'll send a follow-up PR for it once that build comes to Windows Insiders.

Thanks.

josephsl avatar Jul 28 '22 01:07 josephsl

Are we 100% sure that 22621 is SV2 final? It is currently in the external release preview ring but @carlos-zamora seemed to think that 22622 might be the shipped build.

CC @DHowett perhaps for additional clarity?

codeofdusk avatar Jul 28 '22 02:07 codeofdusk

Hi, VS2019 image uses 22000 whereas VS2022 image has upgraded to 22621. Thought the SDK notice was reverted but perhaps not (the latest commit from me was a few minutes ago). Thanks.

josephsl avatar Jul 28 '22 02:07 josephsl

My comment was in regards to Windows version detection.

codeofdusk avatar Jul 28 '22 02:07 codeofdusk

Hi, I’m assuming 22621 as that’s what the SDK says unless the shipping version will indeed be 22622 (beta), and if folks from Terminal team says so, I can either replace WIN11_22H2 entry or add 22622 to winVersion module. I advise holding off on further reviews for this PR until we get a word from Microsoft people as this PR can confuse everyone (not only NV Access and Microsoft, but also folks working on other products including various screen readers and apps). For NV Access folks, build 22622 is an enablement package on top of 22621 with features being tested by beta channel Insiders and UBR (update build revision, the fourth number in major.minor.build.revision) is higher than what release preview Insiders are running. Thanks.

josephsl avatar Jul 28 '22 02:07 josephsl

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • FAIL: System tests. See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/ducbu56qtbh2gk2w/artifacts/output/nvda_snapshot_pr13867-26023,426d8660.exe

See test results for failed build of commit 426d8660fa

AppVeyorBot avatar Jul 28 '22 04:07 AppVeyorBot

Hi,

Major update: build 19045 (Windows 10 Version 22H2) was released to release preview Insiders on July 28, 2022. Therefore a PR to add 19045 will be posted soon, and I think that one can be merged first before this PR (that PR will target 2022.4).

Thanks.

josephsl avatar Jul 28 '22 17:07 josephsl

Blocking #13297.

codeofdusk avatar Jul 29 '22 08:07 codeofdusk

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • FAIL: System tests. See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/0kynf05mcroqyxd1/artifacts/output/nvda_snapshot_pr13867-26048,e777ac75.exe

See test results for failed build of commit e777ac75d8

AppVeyorBot avatar Jul 29 '22 13:07 AppVeyorBot

Given that we haven't heard from @DHowett or @carlos-zamora on the Windows release strategy and I'd really like to see #13297 unblocked to allow for wide testing early in the 2022.4 release cycle, I think it's reasonable to set 22621 as the SV2 build number for now. As 22621 is safely in the SV2 build range and we usually check for "at least" a given build in NVDA tests, even if SV2 final is 22622, NVDA shouldn't be affected. Thoughts @josephsl?

codeofdusk avatar Aug 12 '22 23:08 codeofdusk

Hi, I’ve come to the same conclusion recently, so I’ll take care of that tonight (just need to remove SConstruct commit). Thanks for the follow-up.

josephsl avatar Aug 12 '22 23:08 josephsl

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • Build execution time has reached the maximum allowed time for your plan (60 minutes).

See test results for failed build of commit 78bc83839d

AppVeyorBot avatar Aug 13 '22 01:08 AppVeyorBot

Hey all! Sorry for the radio silence.

22621 is the final version number for SV2. It is my understanding that the update being tested in the Beta channel today will not increment the version number (which means that Beta will stay a higher version number than RTM.)

DHowett avatar Aug 13 '22 01:08 DHowett

Thanks @DHowett for clarifying!

codeofdusk avatar Aug 13 '22 03:08 codeofdusk

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • FAIL: System tests. See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/d2ywgck4fr4jdwqa/artifacts/output/nvda_snapshot_pr13867-26153,78bc8383.exe

See test results for failed build of commit 78bc83839d

AppVeyorBot avatar Aug 15 '22 01:08 AppVeyorBot

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • FAIL: System tests. See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/53obb6ap62w9k185/artifacts/output/nvda_snapshot_pr13867-26156,911e87fe.exe

See test results for failed build of commit 911e87fe9f

AppVeyorBot avatar Aug 15 '22 03:08 AppVeyorBot

Should this have the blocked label removed?

seanbudd avatar Aug 15 '22 05:08 seanbudd

Yes – now that we've heard from @DHowett re the build number and @josephsl has removed unrelated changes, the PR is now ready for merge.

codeofdusk avatar Aug 15 '22 06:08 codeofdusk