8000 MSI installer to perform pre-requisite checks by bergmeister · Pull Request #4602 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]

Skip to content

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

Merged
merged 4 commits into from
Aug 18, 2017

Conversation

bergmeister
Copy link
Contributor
@bergmeister bergmeister commented Aug 16, 2017

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:

  • Universal C Runtime prior to Windows 10: A check for the existence of ucrtbase.dll in the system32 folder is performed (it would be possible to additionally assert against a minimum file version if you wish so)
  • Visual C++ Redistributable for VS2015: Registry entries are checked. The 64 bit installer checks for the 32 and 64 bit version of it whereas the 32 bit installer only asserts against the 32 bit version of the redistributables.

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

…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.
@msftclas
Copy link

@bergmeister,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@adityapatwardhan
Copy link
Member

@bergmeister Thanks!

@TravisEz13 TravisEz13 changed the title #4458 MSI installer to perform pre-requisite checks MSI installer to perform pre-requisite checks Aug 16, 2017
<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">
Copy link
Member

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."

Copy link
Contributor Author

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.

@bergmeister
Copy link
Contributor Author
bergmeister commented Aug 17, 2017

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?

@TravisEz13
Copy link
Member

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.
@TravisEz13
Copy link
Member

@mirichmo can you update your review?

<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">
Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Member
@mirichmo mirichmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@TravisEz13 TravisEz13 merged commit 54e892a into PowerShell:master Aug 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0