Installer: try multiple times to remove the NVDA exes, giving the installed NVDA extra time to fully exit on the UAC screen#16205
Merged
Conversation
* when starting the install, first wait an initial second to definitely give time for the installed NVDA (that may still be exiting on the UAC screen) to fully exit. * When deleting each of the NvDA exes, try up to 6 times for each, with a wait interval of 0.5 seconds. this is similar to what the code used to do. It is possible for the installed NVDA on the secure screen to take some time to exit, so we must give as much chance as possible.
seanbudd
reviewed
Feb 21, 2024
seanbudd
left a comment
Member
There was a problem hiding this comment.
approved pending minor feedback
| installDir: str, | ||
| relativeFilepaths: Iterable[str], | ||
| numTries: int = 6, | ||
| retryWeightInterval: float = 0.5 |
Member
There was a problem hiding this comment.
Suggested change
| retryWeightInterval: float = 0.5 | |
| retryWaitInterval: float = 0.5 |
Member
There was a problem hiding this comment.
can you please update the docstring to cover the new parameters
* Update docstring.
e7906ac to
3a3e4cc
Compare
seanbudd
approved these changes
Feb 25, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Fixes #16199
Fixes #16122
Summary of the issue:
In pr #16174, effort was made to make the installer more robust, including better handling when the installed NVDA is still running, by trying to remove the NVDA exes first, and if any of them fail, restoring them all and not corrupting a user's install.
Although it no longer corrupts the install, it was too fast at at trying to remove the exes and no longer tried multiple times, which did not take into account that the installed NVDA has just been running on the UAC screen, and may still be exiting.
Description of user facing changes
NVDA will try multiple times itself to remove the NVDA exes while waiting for the installed NvDA to fully exit.
Description of development approach
Testing strategy:
Known issues with pull request:
None known.
Code Review Checklist: