-
Notifications
You must be signed in to change notification settings - Fork 7.6k
MSI installer to perform pre-requisite checks #4602
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
MSI installer to perform pre-requisite checks #4602
Conversation
…S Studio 2015 C++ redistributables are present and returns error message if not. Includes tests to check that the Wix file contains the download URLs and those URLs are not dead.
@bergmeister, |
@bergmeister Thanks! |
assets/Product.wxs
Outdated
<RegistrySearch Id="VCRedist_RegSearch_x86" Root="HKLM" Name="Version" Type="raw" Win64="no" | ||
Key="SOFTWARE\Microsoft\DevDiv\VC\Servicing\14.0\RuntimeMinimum"/> | ||
</Property> | ||
<Condition Message="$(env.ProductName) requires because the Universal C Runtime to be installed. You can download it here: https://www.microsoft.com/en-us/download/details.aspx?id=50410"> |
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.
Typo in the error string. Please remove "because."
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.
Well spotted. I fixed it.
Should I also update the link to the Visual Studio C++ redistributes in the pre-requisites documentation for consistency and also because the link in the documentation requires an MSDN su 8000 bscription? |
If you have a good alternative, that would be great. |
… pre-requisites documentation as the current link requires an MSDN subscription and to make the link consistent with the one in the error message by the installer.
@mirichmo can you update your review? |
assets/Product.wxs
Outdated
<Condition Message="$(env.ProductName) requires the Universal C Runtime to be installed. You can download it here: https://www.microsoft.com/en-us/download/details.aspx?id=50410"> | ||
<![CDATA[Installed OR UCRTINSTALLED]]> | ||
</Condition> | ||
<Condition Message="$(env.ProductName) requires the Visual Studio C++ 2015 x64 redistributables to be installed. You can download it here: https://www.microsoft.com/en-gb/download/details.aspx?id=48145"> |
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.
It's close. I overlooked this the first time. Use a link without the language specifier so that it uses the browser's default language. For example:
https://www.microsoft.com/download/details.aspx?id=48145 automatically redirects to en-us for me.
This should also be fixed in the other files as well.
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.
OK, makes sense, especially for non-English speaker. But interestingly when I omit the language specifier, I get redirected to 'en/us' as well and not 'en/gb'. I fixed that in the files that I changed as part of this PR. However, note that the whole source code contains around 52 occurrences of '/en-us' in case you want to raise a separate issue for that.
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 created issue #4610 to cover it.
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.
Thanks
Implemented #4458: Checks that the following pre-requisites of PowerShell Core on Windows are met by displaying an error message that includes a download link:
The ideas against what to check comes from research and especially this example here to which I was redirected in a WiX discussion.
Includes tests to check that the WiX file contains the download URLs and those URLs are not dead.
I'll have to leave the extensive testing on different systems up to you. Maybe it is also best if you double check the used assertions with the right experts at MSFT.
NB: Due to recent refactorings of the VC++ team (see blog post here) it has become very complicated to include those binaries and therefore a check against them should be the minimum viable solution as originally outlined by @SteveL-MSFT