Include the Desktop UWP redistributables in NVDA for Onecore features rather than the Onecore redistributables#8003
Merged
Merged
Conversation
…, rather than the Onecore redistributables. This should fix issues on windows 7.
Member
Author
|
Once the reporter of #7975 confirms the bug is fixed, and this has been approved, it should go straight to master for 2018.1. |
Member
Author
|
This build should be tested on a clean install of Windows 7 sp1 as well. |
Contributor
|
Hi, requesting P1 for this. Thanks.
From: Michael Curran [mailto:notifications@github.com]
Sent: Thursday, February 15, 2018 11:57 AM
To: nvaccess/nvda <nvda@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Subject: [nvaccess/nvda] Include the Desktop UWP redistributables in NVDA for Onecore features rather than the Onecore redistributables (#8003)
Link to issue number:
Fixes #7975 <#7975>
Summary of the issue:
NVDA fails to run on Windows 7 when a certain version of the Visual Studio 2017 Redistributables are installed.
This is due to a conflict between our copies and the installed copies.
Currently we bundle the redistributables in order to support Onecore Voices and OCR on Windows 10 systems that do not yet come with redistributibles 14.1 or higher (such as Windows 10 Nov 2015 and earlier).
However, it seems we have been copying the wrong redistributables into our build. Two copies of these dlls come with VS 2017, We were copying in the "Onecore" ones rather than the Desktop ones, under the assumption that we needed the onecore ones to access onecore features.
Testing shows this is not necessary. The Desktop ones allow access to onecore features, and removes the errors on Windows 7 due to the conflict.
Description of how this pull request fixes the issue:
We now copy the Desktop msvcp vccorelib and vcruntime dlls, rather than the Onecore ones.
Testing performed:
Tested NVDA on Windows 10 Nov 2015 (with no existing redistributables). Can use windows Onecore voices. Deleting the dlls from our build makes the Onecore synth driver fail, thus NVDA is successfully making use of them.
Tested NVDA on Windows 10 insider build 17093. Windows Onecore voices continue to work.
At least one user who was experiencing the error on Windows 7 has tested a build with the correct redistributables and reports the errors have gone away.
Known issues with pull request:
None
Change log entry:
Bug fixes:
* NVDA no longer fails to start on Windows 7 complaining about an internal api-ms dll., when a particular version of the Visual Studio 2017 redistributables have been installed by another application.
…_____
You can view, comment on, or merge this pull request online at:
#8003
Commit Summary
* Include the Desktop UWP redistributables in NVDA for Onecore features, rather than the Onecore redistributables. This should fix issues on windows 7.
File Changes
* M nvdaHelper/localWin10/sconscript <https://github.com/nvaccess/nvda/pull/8003/files#diff-0> (2)
Patch Links:
* https://github.com/nvaccess/nvda/pull/8003.patch
* https://github.com/nvaccess/nvda/pull/8003.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub <#8003> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AHgLkIoukq0lhPDygzeBEkcvCe2bCoaDks5tVIwZgaJpZM4SHasB> .
|
Member
Author
|
Yep. Both the issue and PR have the p1 label :)
|
|
I wonder what that application is though, I obviously do not have the issue
here. When the new version is made will it delete the bad redistributable
for existing users too in case they install whatever it was that put in the
older version?
Brian
|
|
Is there a link for the test build?
Brian
|
Member
Author
|
The good and bad dlls have the same names, thus the new ones will simply
overwrite the old ones.
|
Member
Author
Contributor
|
It works correctly on win 10, 1709
Wysłane z aplikacji Poczta dla Windows 10
Od: Michael Curran
Wysłano: czwartek, 15 lutego 2018 22:50
Do: nvaccess/nvda
DW: Subscribed
Temat: Re: [nvaccess/nvda] Include the Desktop UWP redistributables in NVDAfor Onecore features rather than the Onecore redistributables (#8003)
Try build: https://ci.appveyor.com/api/buildjobs/ud81umqv4k7i6j2p/artifacts/output%2Fnvda_snapshot_try-i7975-14863%2C6a93ee31.exe
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Noted they have the same names, strange choice really. I have however tested
all ways I can think of on windows 7 with no ill effects as far as I'm
concerned. it, I assume fixed the original reporters problem, did it. As I
say, I could not get the fault to happen here but mine was a new install of
windows 7 in 2015, and I've never messed with it since.
Brian
|
Member
Author
|
This has been tested by several people, including by 2 who were directly affected by the issue. This has been merged to master before rc later this week due to its high impact, and it is only a one line change. |
feerrenrut
reviewed
Mar 6, 2018
feerrenrut
left a comment
Contributor
There was a problem hiding this comment.
Looks good, glad a solution was found for this.
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
6 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 #7975
Summary of the issue:
NVDA fails to run on Windows 7 when a certain version of the Visual Studio 2017 Redistributables are installed.
This is due to a conflict between our copies and the installed copies.
Currently we bundle the redistributables in order to support Onecore Voices and OCR on Windows 10 systems that do not yet come with redistributibles 14.1 or higher (such as Windows 10 Nov 2015 and earlier).
However, it seems we have been copying the wrong redistributables into our build. Two copies of these dlls come with VS 2017, We were copying in the "Onecore" ones rather than the Desktop ones, under the assumption that we needed the onecore ones to access onecore features.
Testing shows this is not necessary. The Desktop ones allow access to onecore features, and removes the errors on Windows 7 due to the conflict.
Description of how this pull request fixes the issue:
We now copy the Desktop msvcp vccorelib and vcruntime dlls, rather than the Onecore ones.
Testing performed:
Tested NVDA on Windows 10 Nov 2015 (with no existing redistributables). Can use windows Onecore voices. Deleting the dlls from our build makes the Onecore synth driver fail, thus NVDA is successfully making use of them.
Tested NVDA on Windows 10 insider build 17093. Windows Onecore voices continue to work.
At least one user who was experiencing the error on Windows 7 has tested a build with the correct redistributables and reports the errors have gone away.
Known issues with pull request:
None
Change log entry:
Bug fixes: