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

Fix the prerequisite check of MSI package #5070

Merged
merged 8 commits into from Oct 10, 2017

Conversation

Projects
None yet
6 participants
@daxian-dbw
Member

daxian-dbw commented Oct 9, 2017

Fix #5029

The Fix

  • Remove the check for Visual C++ Redistributable
  • Add a precheck for WMF 4.0 on Win7.

pwrshplugin.dll has the following dependencies. All the api-ms-win-crt-* api sets are part of the Universal C Runtime. The plugin DLL doesn't depend on Visual C++ Redistributable.

    KERNEL32.dll
    ole32.dll
    WsmSvc.DLL
    api-ms-win-crt-string-l1-1-0.dll
    api-ms-win-crt-convert-l1-1-0.dll
    api-ms-win-crt-heap-l1-1-0.dll
    api-ms-win-crt-runtime-l1-1-0.dll
    api-ms-win-crt-stdio-l1-1-0.dll
    api-ms-win-crt-filesystem-l1-1-0.dll
    ADVAPI32.dll
    api-ms-win-crt-locale-l1-1-0.dll
    api-ms-win-crt-multibyte-l1-1-0.dll

On Win7, WMF 4.0 or higher version needs to be installed so that the WinRM can support the side-by-side remoting plugin. So I make the WMF 4.0 a prerequisite too.
The way to check PSv4 or above is to check the file version of \windows\system32\pwrshplugin.dll. The file version for in-box PSv4 is 6.3.9600.16384, for WMF 4.0 is 6.3.9600.16406, and for PSv5+ is 10.0.xxx. So we can check the MinVersion of the file pwrshplugin.dll to be 6.3.9600.16383 (means the version needs to be at least 6.3.9600.16384).

I have verified that on 2012R2, Win8.1 and Win7 once Universal C Runtime is installed (it requires a lot of other KB's to be installed) and PSv4 or WMF 4 is available, PowerShell remoting works.

Screenshot Examples

On a machine that doesn't have Universal C Runtime installed:
image

On Win7 that has Universal C Runtime installed but no WMF 4 or above
image

Show outdated Hide outdated assets/Product.wxs Outdated
Show outdated Hide outdated assets/Product.wxs Outdated
Show outdated Hide outdated docs/installation/windows.md Outdated
Show outdated Hide outdated assets/Product.wxs Outdated
@iSazonov

This comment has been minimized.

Show comment
Hide comment
@iSazonov

iSazonov Oct 10, 2017

Collaborator

Interesting, can an user copy the link we show on error window? Can we do the link clickable?

Collaborator

iSazonov commented Oct 10, 2017

Interesting, can an user copy the link we show on error window? Can we do the link clickable?

@SteveL-MSFT

This comment has been minimized.

Show comment
Hide comment
@SteveL-MSFT

SteveL-MSFT Oct 10, 2017

Member

@iSazonov unfortunately, the text the installer displays is not clickable or copyable. Will have to research wix to figure out how to have a custom window that has a clickable link.

Member

SteveL-MSFT commented Oct 10, 2017

@iSazonov unfortunately, the text the installer displays is not clickable or copyable. Will have to research wix to figure out how to have a custom window that has a clickable link.

@daxian-dbw

This comment has been minimized.

Show comment
Hide comment
@daxian-dbw

daxian-dbw Oct 10, 2017

Member

@iSazonov it's good that we are now using the short URL "http://aka.ms/pscore6-prereq". It mitigates the problem to some extent.

Member

daxian-dbw commented Oct 10, 2017

@iSazonov it's good that we are now using the short URL "http://aka.ms/pscore6-prereq". It mitigates the problem to some extent.

@iSazonov

This comment has been minimized.

Show comment
Hide comment
@iSazonov
Collaborator

iSazonov commented Oct 10, 2017

@SteveL-MSFT

This comment has been minimized.

Show comment
Hide comment
@SteveL-MSFT

SteveL-MSFT Oct 10, 2017

Member

Perhaps we should accept this PR as-is (barring any other concerns) and have a new PR/issue for the custom hyperlink control?

Member

SteveL-MSFT commented Oct 10, 2017

Perhaps we should accept this PR as-is (barring any other concerns) and have a new PR/issue for the custom hyperlink control?

@iSazonov

This comment has been minimized.

Show comment
Hide comment
@iSazonov

iSazonov Oct 10, 2017

Collaborator

We can not rush to the next Beta.9 release.

Collaborator

iSazonov commented Oct 10, 2017

We can not rush to the next Beta.9 release.

Show outdated Hide outdated assets/Product.wxs Outdated
Show outdated Hide outdated assets/Product.wxs Outdated
Show outdated Hide outdated assets/Product.wxs Outdated
@bergmeister

This comment has been minimized.

Show comment
Hide comment
@bergmeister

bergmeister Oct 10, 2017

Contributor

As far as I have seen the message dialog does not allow hyperlinks but the WiX Cookbook gives examples on how to deal with hyperlinks in chapter 11:

One could add a custom action that opens the browser if the installation failed due to the pre-requisite check or embed the hyperlink in a custom WiX exit dialogue in case of failure.

Contributor

bergmeister commented Oct 10, 2017

As far as I have seen the message dialog does not allow hyperlinks but the WiX Cookbook gives examples on how to deal with hyperlinks in chapter 11:

One could add a custom action that opens the browser if the installation failed due to the pre-requisite check or embed the hyperlink in a custom WiX exit dialogue in case of failure.

@daxian-dbw

This comment has been minimized.

Show comment
Hide comment
@daxian-dbw

daxian-dbw Oct 10, 2017

Member

@adityapatwardhan and @bergmeister Thanks for your feedback and I have addressed them. Please take another look.

Member

daxian-dbw commented Oct 10, 2017

@adityapatwardhan and @bergmeister Thanks for your feedback and I have addressed them. Please take another look.

Show outdated Hide outdated assets/Product.wxs Outdated
Show outdated Hide outdated docs/installation/windows.md Outdated
@bergmeister

This comment has been minimized.

Show comment
Hide comment
@bergmeister

bergmeister Oct 10, 2017

Contributor

@daxian-dbw Should we also add the new WMF download URLS to the installer tests as their purpose is simply to check that links are still working?

Contributor

bergmeister commented Oct 10, 2017

@daxian-dbw Should we also add the new WMF download URLS to the installer tests as their purpose is simply to check that links are still working?

@adityapatwardhan

This comment has been minimized.

Show comment
Hide comment
@adityapatwardhan

adityapatwardhan Oct 10, 2017

Member

@daxian-dbw Please use [Feature] in the last git commit message so that the modified test is executed.

Member

adityapatwardhan commented Oct 10, 2017

@daxian-dbw Please use [Feature] in the last git commit message so that the modified test is executed.

@daxian-dbw

This comment has been minimized.

Show comment
Hide comment
@daxian-dbw

daxian-dbw Oct 10, 2017

Member

@bergmeister new tests are added to verify the WMF download URLs
@adityapatwardhan the latest commit was pushed with [Feature] tag.

Member

daxian-dbw commented Oct 10, 2017

@bergmeister new tests are added to verify the WMF download URLs
@adityapatwardhan the latest commit was pushed with [Feature] tag.

@bergmeister

This comment has been minimized.

Show comment
Hide comment
@bergmeister

bergmeister Oct 10, 2017

Contributor

LGTM

Contributor

bergmeister commented Oct 10, 2017

LGTM

@adityapatwardhan adityapatwardhan merged commit f7bab36 into PowerShell:master Oct 10, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details

@daxian-dbw daxian-dbw deleted the daxian-dbw:fixmsi branch Oct 11, 2017

powercode pushed a commit to powercode/PowerShell that referenced this pull request Oct 19, 2017

Fix the prerequisite check of MSI package (PowerShell#5070)
* Skip check 'vsruntime140.dll' on Win10

* Make the precheck accurate

* Update windows installation prerequisites

* Change 'higher' to 'above'

* Address comments

* Address more comments

* Address some more comments

* [Feature] Update the installer test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment