GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — MSI installer to perform pre-requisite checks by bergmeister · Pull Request #4602 · PowerShell/PowerShell

Conversation

@bergmeister

@bergmeister Christoph Bergmeister (bergmeister) commented Aug 16, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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 Steve Lee (@SteveL-MSFT)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
daxian-dbw reacted with hooray emoji
All reactions
  • 🎉 1 reaction
@bergmeister
PowerShell#4458 MSI installer checks that the Windows C Runtime and V…
a354e6c 
…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

Microsoft Contribution License Agreements (msftclas) commented Aug 16, 2017

Copy link
Copy Markdown

Christoph Bergmeister (@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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-already-signed label Aug 16, 2017
@bergmeister Christoph Bergmeister (bergmeister) mentioned this pull request Aug 16, 2017
MSI package should perform pre-req checks #4458
Closed
@adityapatwardhan
Aditya Patwardhan (adityapatwardhan) requested review from Steve Lee (SteveL-MSFT) and Raghu Shantha (raghushantha) August 16, 2017 23:03
@adityapatwardhan

Aditya Patwardhan (adityapatwardhan) commented Aug 16, 2017

Copy link
Copy Markdown
Member

Christoph Bergmeister (@bergmeister) Thanks!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@daxian-dbw Dongbo Wang (daxian-dbw) assigned Travis Plunk (TravisEz13) Aug 16, 2017
@TravisEz13 Travis Plunk (TravisEz13) changed the title #4458 MSI installer to perform pre-requisite checks MSI installer to perform pre-requisite checks Aug 16, 2017
mirichmo
Mike Richmond (mirichmo) suggested changes Aug 17, 2017
View reviewed changes
Comment thread 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">

@mirichmo Mike Richmond (mirichmo) Aug 17, 2017

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@bergmeister Christoph Bergmeister (bergmeister) Aug 17, 2017

Copy link
Copy Markdown
Contributor Author

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bergmeister
PowerShell#4458 Fixed simple typo in error message spotted during cod…
a0fde6e 
…e review of PR PowerShell#4602
@bergmeister

Christoph Bergmeister (bergmeister) commented Aug 17, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

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 subscription?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@TravisEz13

Travis Plunk (TravisEz13) commented Aug 17, 2017

Copy link
Copy Markdown
Member

If you have a good alternative, that would be great.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bergmeister
Update download link to Visual Studio C++ redistributables in Windows…
87611d3 
… 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

Travis Plunk (TravisEz13) commented Aug 17, 2017

Copy link
Copy Markdown
Member

Mike Richmond (@mirichmo) can you update your review?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
mirichmo
Mike Richmond (mirichmo) suggested changes Aug 17, 2017
View reviewed changes
Comment thread 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">

@mirichmo Mike Richmond (mirichmo) Aug 17, 2017

Copy link
Copy Markdown
Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@bergmeister Christoph Bergmeister (bergmeister) Aug 17, 2017

Copy link
Copy Markdown
Contributor Author

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mirichmo Mike Richmond (mirichmo) Aug 17, 2017

Copy link
Copy Markdown
Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bergmeister
PowerShell#4458 Removed language identifiers in URLs as requested by …
f5b7dc2 
…code review in PR PowerShell#4602
mirichmo
Mike Richmond (mirichmo) approved these changes Aug 17, 2017
View reviewed changes

@mirichmo Mike Richmond (mirichmo) left a comment

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@TravisEz13
Travis Plunk (TravisEz13) merged commit 54e892a into PowerShell:master Aug 18, 2017
@iSazonov Ilya (iSazonov) mentioned this pull request Aug 25, 2017
WindowsInstaller.Tests.ps1 failed on debian #4663
Closed
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@bergmeister @TravisEz13
Make MSI installer perform pre-requisite checks (PowerShell#4602)
247dd17 
* PowerShell#4458 MSI installer checks that the Windows C Runtime and VS 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@raghushantha Raghu Shantha (raghushantha) Awaiting requested review from raghushantha
@SteveL-MSFT Steve Lee (SteveL-MSFT) Awaiting requested review from SteveL-MSFT
1 more reviewer
@mirichmo Mike Richmond (mirichmo) mirichmo approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@TravisEz13 Travis Plunk (TravisEz13)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@bergmeister @msftclas @adityapatwardhan @TravisEz13 @mirichmo
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.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.