nvda icon indicating copy to clipboard operation
nvda copied to clipboard

Make nvdaHelper and microsoft-ui-uiautomation pre-built dependencies

Open seanbudd opened this issue 1 year ago • 9 comments

Is your feature request related to a problem? Please describe.

Building NVDA on AppVeyor is getting slower and slower. It appears that AppVeyor's build environment is getting slower, as similar build actions are taking longer times. The total build time is 40min+ now. We have a maximum of 60min.

Building microsoft-ui-uiautomation\src\uiAutomation\UIAutomation.sln takes 12 minutes. Building nvdaHelper also takes a long time.

[00:08:06] Start Source build
[00:08:39] Start build microsoft-ui-uiautomation\src\uiAutomation\UIAutomation.sln 
[00:21:37] Finish build MS UIA.sln Time Elapsed 00:12:54.18
[00:25:51] Finish source build, start launcher devguide changes userguide client appx
[00:29:47] Start checkPot

Describe the solution you'd like

Pre-build microsoft-ui-uiautomation\src\uiAutomation\UIAutomation.sln and pull it in as a prebuilt dependency.

Build nvdaHelper in a separate repository. Pull it in as a prebuilt dependency. This will remove the need to use certain Visual Studio build tools for general NVDA development. This will be a non-trivial task as we will have to split up the build documentation and process.

I propose using GitHub actions to build nvdaHelper. This will require an investigation into the minutes NVDA can use and a projection of minutes we would use. We may want to schedule a regular build and update to NVDA alpha to make use of the latest updates to the compiler on GitHub actions.

Describe alternatives you've considered

Splitting out a different part of the AppVeyor build process (e.g. system tests)

Additional context

seanbudd avatar Aug 08 '22 06:08 seanbudd

Pre-build microsoft-ui-uiautomation\src\uiAutomation\UIAutomation.sln and pull it in as a prebuilt dependency.

+100 to this. Given how long it takes to build, and that building it locally still fails for some people depending of the VS version in use that would be huge plus, even for external contributors.

Build nvdaHelper in a separate repository. Pull it in as a prebuilt dependency. This will remove the need to use certain Visual Studio build tools for general NVDA development. This will be a non-trivial task as we will have to split up the build documentation and process.

I'm not too sure about this. While NVDAHelper is not modified with every PR it is a pretty integral part of NVDA, and it still receives semi regular patches, which pretty commonly depend on changes to the Python source. Wouldn't it be unnecessarily difficult to review PR's modifying NVDAHelper if it is kept in a separate repository?

I propose using GitHub actions to build nvdaHelper. This will require an investigation into the minutes NVDA can use and a projection of minutes we would use. We may want to schedule a regular build and update to NVDA alpha to make use of the latest updates to the compiler on GitHub actions.

Any particular reason for changing the CI provider? While ApppVeyor certainly has some limitations it is proven to work for our use cases.

Splitting out a different part of the AppVeyor build process (e.g. system tests)

I'm not sure how feasible this idea is. As I see the problem we can gain time only by making build process finish sooner. If system tests are executed separately we would have to wait for their results before marking the build as pass or fail. An alternative idea would be to execute system tests in a separate AppVeyor job - while this wouldn't make build faster we would no longer be restricted by the 60 minutes time limit.

It would also be worth defining what build time we consider to be short enough. Having a concrete time goal may let us to start by moving build of UIA Remote library to a separate repository, and then considering if we need to split up the build process further.

lukaszgo1 avatar Aug 09 '22 17:08 lukaszgo1

Pre-build microsoft-ui-uiautomation\src\uiAutomation\UIAutomation.sln and pull it in as a prebuilt dependency.

This should be the first step, as it has the quickest and biggest gains.

I'm not too sure about this. While NVDAHelper is not modified with every PR it is a pretty integral part of NVDA, and it still receives semi regular patches, which pretty commonly depend on changes to the Python source. Wouldn't it be unnecessarily difficult to review PR's modifying NVDAHelper if it is kept in a separate repository?

I don't think this should be a major issue if we get the development pipeline right. The value of increased modularity and simpler NVDA builds for most PRs outweigh the whatever added maintenance there is ideally.

Any particular reason for changing the CI provider? While AppVeyor certainly has some limitations it is proven to work for our use cases.

There are quite a few. The task to investigate and consider migrating should discuss it in better details.

seanbudd avatar Aug 10 '22 07:08 seanbudd

@mazen428 - out of curiosity, is there a reason for the "thumbs down" emoji?

seanbudd avatar Aug 10 '22 07:08 seanbudd

It would also be worth defining what build time we consider to be short enough. Having a concrete time goal may let us to start by moving build of UIA Remote library to a separate repository, and then considering if we need to split up the build process further.

I think keeping it under 30min is ideal.

seanbudd avatar Aug 10 '22 07:08 seanbudd

I'm not sure how feasible this idea is. ... An alternative idea would be to execute system tests in a separate AppVeyor job - while this wouldn't make build faster we would no longer be restricted by the 60 minutes time limit.

This is what I meant by

Splitting out a different part of the AppVeyor build process (e.g. system tests)

GitHub actions would also allow us to run unit tests, translation comment check and the lint simultaneously to the system tests.

seanbudd avatar Aug 10 '22 07:08 seanbudd

A quick solution that (hopefully) minimizes complexity might be to cache the intermediate steps of the build: https://www.appveyor.com/docs/build-cache/

It's tempting to get drawn in by benefits like community developers no longer requiring Visual studio to modify NVDA, while this work may move towards that, the primary concern is staying within the max allowed build time.

To ensure we spend time optimizing for the biggest returns, a more thorough profile of the current appveyor timing for the components and steps should be collected, along with descriptions of the approaches and an estimate for the reduction of time.

feerrenrut avatar Aug 10 '22 13:08 feerrenrut

The current problem of appveyor limiting builds is quite serious, as long as there are two or more builds in progress, you have to wait a long time to update the version that needs to be tested. Perhaps we could use github actions to build NVDA Of course you can also use wlixcc/SFTP-Deploy-Action Finally, upload the generated build to the server

dpy013 avatar Aug 22 '22 09:08 dpy013

Note that #14235 attempted to build microsoft-ui-uiautomation externally, but it was found not to be a viable approach. Alternatives may be:

  • On appveyor, cache build outputs for microsoft-ui-uiautomation.
    • Ensure the process also works for dev builds.
    • Consider dependencies that should trigger a rebuild of microsoft-ui-uiautomation (build script, changed submodule)
    • Ensure that using cache doesn't result in an unnecessary rebuild of microsoft-ui-uiautomation (use hash rather than timestamp).
  • Incorporate creation of UIARemote.dll (nvdaHelper/UIARemote/sconscript) into nvda-microsoft-ui-uiautomation
    • No other NVDA code directly depends on microsoft-ui-uiautomation headers (first confirm)

feerrenrut avatar Oct 12 '22 05:10 feerrenrut