-
-
Notifications
You must be signed in to change notification settings - Fork 659
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
Restore VS2022 support #13524
Restore VS2022 support #13524
Conversation
@michaelDCurran - any idea on a testing strategy here? |
As far as I know, the only differences in code between the situation before and after this pr consist of review actions by @michaelDCurran on microsoft/Microsoft-UI-UIAutomation#84 |
The fact that I required Visual Studio 2019 on both my systems while I am using VS2022 for personal work really frustrated me. After several hours of research, I found the solution 🙌🏻 It's now part of this pr as well. |
This reverts commit 31aa5eb.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would much prefer that these were two separate PRs.
May I suggest you keep this one for reintroducing support for VS 2022, and move the submodule change to a separate pr.
However, it is worth noting that I have several PRs open on microsoft/microsoft-ui-uiautomation so there may be future points where my fork is still necessary. At very least, the pr to switch the submodule is perhaps not as urgent or necessary for wide testing as the reintroduction of VS 2022.
But, I'm really glad you were able to track down the issue :)
sconstruct
Outdated
@@ -125,7 +125,7 @@ env = Environment(variables=vars,HOST_ARCH='x86',tools=[ | |||
# Specifically request to use Visual Studio 2019 (MSVC 14.2). | |||
# This will ensure that this specific version is selected if there are multiple versions installed, | |||
# And fail the build if this version is not installed at all. | |||
env.SetDefault(MSVC_VERSION='14.2') | |||
#env.SetDefault(MSVC_VERSION='14.2') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please delete this line and the comment above.
But, we should also reintroduce code into nvdaHelper/archBuild_sconscript to ensure the chosen msvc version is 14.2 or higher.
@michaelDCurran Thanks, I addressed your comments. |
I'm happy to confirm / test this.
But in short:
With MS word 16.0.15000 or higher:
* Open MS word
* Type several lines of text.
* Insert a section break between two of the lines. Confirm that when
arrowing up and down over the section break NVDA announces the changing
section number
* Insert a bookmark at some point in the text. Read the line with the
bookmark and confirm that NVDA announces bookmark at the start of the
bookmark.
* Turn on report line numbers in NVDA's document formatting settings.
Arrow up and down the lines and confirm that NVDA is nnouncing the line
numbers.
|
@michaelDCurran - is this the right issue/PR? do we need to (re)open an issue? |
@seanbudd Err, certainly is not related to this pr. In fact, I tracked down that comment of mine... it was on pr #13524 on March 23. I have no idea why it shows up here and says it was sent 17 hours ago. Based on other github eails in the last 12 hours seeming to be on old issues... something might be up with Github and timestamps... Very odd. |
Oh... it is the right pr number, just the wrong date. |
Link to issue number:
Fix-up of #13387
Fixes #13033
Summary of the issue:
Pr #13387 introduced support for custom UIA extensions, there by using the Microsoft UIA Automation remote operations library. This pr disabled building support for Visual Studio 2022. This has now been solved.
Description of how this pull request fixes the issue:
Fixes build on Visual Studio 2022 by ensuring that the windir environment variable is available. This should eventually be fixed in scons itself, though there's a note in their source code stating that adding environment variables to the basic set that's imported from os.environ should be considered very carefully.
Testing strategy:
Ensured NVDA builds with Visual Studio 2022 and runs properly.
Known issues with pull request:
I tried overriding the platform toolset defined in the vcxproj files with the current toolset as used by SCons for everything else. This would ensure that Visual Studio 2022 uses the V143 toolset (i.e. it would no longer require an explicit install of V142), whereas an update of the UIA libraries that would update the projects to the V143 wouldn't break our build on VS2019. It looks like essential parts of WinRT are also built with VS2019 and therefore this trategy might work, but results in a much slower build. Therefore I explicitly noted the requirement of the V142 (2019) toolset on vs 2022.
Change log entries:
For Developers
Code Review Checklist: