winget-cli icon indicating copy to clipboard operation
winget-cli copied to clipboard

Better handling for MSI Error 1638

Open denelon opened this issue 3 years ago • 3 comments

Brief description of your issue

MSI Error 1638 occurs when I attempt to install over a newer version.

https://docs.microsoft.com/en-us/windows/win32/msi/preventing-an-old-package-from-installing-over-a-newer-version

Steps to reproduce

Try to install an MSI based package when a newer version is already installed.

Expected behavior

I should be informed the package I am attempting to install is an older version.

Actual behavior

The package is downloaded and executed, and I get an error 1638

Environment

PS C:\Users\denelon> winget --info
Windows Package Manager (Preview) v1.3.431-preview
Copyright (c) Microsoft Corporation. All rights reserved.

Windows: Windows.Desktop v10.0.22557.1
Package: Microsoft.DesktopAppInstaller v1.18.431.0

denelon avatar Feb 23 '22 22:02 denelon

@denelon - This seems to already be handled? Perhaps just the message needs to be made more clear?

https://github.com/microsoft/winget-cli/blob/a1df041e867d546fe64685ffcf993c7566e45fe6/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp#L612

Returns the message - https://github.com/microsoft/winget-cli/blob/a1df041e867d546fe64685ffcf993c7566e45fe6/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw#L1276-L1277

Trenly avatar Jan 10 '23 03:01 Trenly

@Trenly,

We did update the MSI code path. Thanks for flagging this. Do you have any suggestions for better messages?

Perhaps:

<Package Name> <Package Version> is currently installed. Installing <Package Name> <"Target" Version> requires uninstalling the current version first.

denelon avatar Jan 10 '23 16:01 denelon

@Trenly,

We did update the MSI code path. Thanks for flagging this. Do you have any suggestions for better messages?

Perhaps:

<Package Name> <Package Version> is currently installed. Installing <Package Name> <"Target" Version> requires uninstalling the current version first.

I think if you can get that level of verbosity in a failure message, it would be great. However, in the current framework I believe that would require a change to how the error gets propagated through and handled. Looking at it, it seems to me that the error is simply passed along, with the installer return code being mapped to an expected return code, then the expected return code being mapped to a CLI error, and the CLI error having the InstallFlowReturnCodeAlreadyInstalled error message.

It should be possible to use the dynamic parameters, they would just have to be passed along properly.

Trenly avatar Jan 10 '23 17:01 Trenly