Skip to content
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

COM API "upgrade" not working when MinimumOSVersion is set in App Manifest! #4589

Closed
PatrickSchmidtSE opened this issue Jun 27, 2024 · 13 comments
Labels
Area-COM-API Issue related to COM API Command-Upgrade Issue related to WinGet Upgrade Issue-Bug It either shouldn't be doing this or needs an investigation.
Milestone

Comments

@PatrickSchmidtSE
Copy link

PatrickSchmidtSE commented Jun 27, 2024

Brief description of your issue

When using the COM-API ( based on this project https://github.com/marticliment/WinGet-API-from-CSharp/tree/main/WindowsPackageManager%20Interop/WindowsPackageManager)
installation etc is working fine. Binaries coming from the project.

But "upgrading" via UpgradePackageAsync has been a very poor experience so far.
Most of the upgrades fail with "NoApplicableInstaller" error code, while winget.exe upgrade does upgrade successfully.

I tested it with many packages, some work (Docker.DockerDesktop) , but many does lead to the above error ( like Java8 etc )

Steps to reproduce

Install a package like "snakefoot.snaketail" in an older version "1.9.7.0"

Then use the COM API to run the UpgradePackageAsync mechanic.

image

Expected behavior

Successfully upgrade to application with InstallResultStatus.OK

Actual behavior

InstallResultStatus.NoApplicableInstaller
image
By the documentation this means, that means that the Upgrade function could not find an available installer given the "options". i tried every options combination, architecture stuff, used all the switches to ignore stuff. Everyhing leads to the same result.
I looked up via the installedVersion property which "Installer" is used and what characteristiks but they are all normal and even setting stuff to the settings the installedVersion has.. It all ends the same.

If i just put "winget upgrade --id snakefoot.snaketail --silent --force" it instantly downloads the msi and installs it successfully.

Proposed problem and solution
So i think i got finally an idea what is happening:

Snakefoot has a MinimumOSVersion included

ManifestVersion: 1.6.0
MinimumOSVersion: 10.0.0.0
PackageIdentifier: snakefoot.snaketail

If you go to this check ---> (sadly OSFilter is NOT publicly changable )

image

You will see that the relevant function is using verifyversioninfow , which delivers a wrong comparism under WIndows10 if the manifest is not changed
image

If i change that in my app manifest it works for my code, but not for the COM or detection from winCLI via COM.
image

https://learn.microsoft.com/de-de/windows/win32/api/winbase/nf-winbase-verifyversioninfow

This must be fixed OR at least adressed via an OSFIlter ignore option :)

Environment

Windows Package Manager (Preview) v1.9.1763-preview
Copyright (c) Microsoft Corporation. All rights reserved.

Windows: Windows.Desktop v10.0.19045.4529
System Architecture: X64

Winget Directories
------------------------------------------------------------------------------------------------------
Logs                               %TEMP%\WinGet\defaultState
User Settings                      %LOCALAPPDATA%\Microsoft\WinGet\Settings\defaultState\settings.json
Portable Links Directory (User)    %LOCALAPPDATA%\Microsoft\WinGet\Links
Portable Links Directory (Machine) C:\Program Files\WinGet\Links
Portable Package Root (User)       %LOCALAPPDATA%\Microsoft\WinGet\Packages
Portable Package Root              C:\Program Files\WinGet\Packages
Portable Package Root (x86)        C:\Program Files (x86)\WinGet\Packages
Installer Downloads                %USERPROFILE%\Downloads

Links
---------------------------------------------------------------------------
Privacy Statement   https://aka.ms/winget-privacy
License Agreement   https://aka.ms/winget-license
Third Party Notices https://aka.ms/winget-3rdPartyNotice
Homepage            https://aka.ms/winget
Windows Store Terms https://www.microsoft.com/en-us/storedocs/terms-of-sale

Admin Setting                             State
--------------------------------------------------
LocalManifestFiles                        Disabled
BypassCertificatePinningForMicrosoftStore Disabled
InstallerHashOverride                     Disabled
LocalArchiveMalwareScanOverride           Disabled
ProxyCommandLineOptions                   Disabled
DefaultProxy                              Disabled
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Triage Issue need to be triaged label Jun 27, 2024
@PatrickSchmidtSE
Copy link
Author

@JohnMcPMS thats the issue for my comment yesterday :)

@denelon denelon added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-COM-API Issue related to COM API Command-Upgrade Issue related to WinGet Upgrade and removed Needs-Triage Issue need to be triaged labels Jun 27, 2024
@PatrickSchmidtSE
Copy link
Author

PatrickSchmidtSE commented Jun 28, 2024

This would also help for analyzing 🗡️

/// a lot of fields and no one requesting it. <<< I DOES NOW :D

/// DESIGN NOTE:
/// GetManifest from IPackageVersion in AppInstallerRepositorySearch is not implemented in V1. That class has
/// a lot of fields andc
/// Gets the manifest of this package version.
/// virtual Manifest::Manifest GetManifest() = 0;

@PatrickSchmidtSE
Copy link
Author

PatrickSchmidtSE commented Jun 28, 2024

So i think i got finally an idea what is happening: @JohnMcPMS This should be an easy fix for you and your team for the next release :)

Snakefoot has a MinimumOSVersion included

ManifestVersion: 1.6.0
MinimumOSVersion: 10.0.0.0
PackageIdentifier: snakefoot.snaketail

If you go to this check ---> (sadly OSFilter is NOT publicly changable )

image

You will see that the relevant function is using verifyversioninfow , which delivers a wrong comparism under WIndows10 if the manifest is not changed
image

If i change that in my app manifest it works for my code, but not for the COM or detection from winCLI via COM.
image

https://learn.microsoft.com/de-de/windows/win32/api/winbase/nf-winbase-verifyversioninfow

This must be fixed OR at least adressed via an OSFIlter ignore option :)

Packages that are working like VIM has NO MinOS on MetaData

image

@PatrickSchmidtSE PatrickSchmidtSE changed the title COM API "upgrade" success rate vs CLI is bad (NoApplicableInstaller) COM API "upgrade" not working when MinimumOSVersion is set in App Manifest! Jun 28, 2024
@JohnMcPMS
Copy link
Member

I'm not sure exactly what is happening in your setup, but based on the data provided I'm guessing that you are using WindowsPackageManagerElevatedFactory, which is surprisingly working from SYSTEM. It is running WindowsPackageManagerServer.exe in an unsupported way, which causes the fact that it does not have the manifested OS support to negatively impact you.

I can add the manifest, but I also recently changed the way that the code underlying that factory works. I would be very surprised if it continues to work from SYSTEM.

The supported way to use winget COM from SYSTEM is with the in-process DLL. Using that, you would have control over the behavior of the version API via the manifest in your own executable.

@PatrickSchmidtSE
Copy link
Author

PatrickSchmidtSE commented Jul 2, 2024

@JohnMcPMS
yes i do :) It works due to several workarounds implemented :) i know its unsupported, we all wait for official support of winget for SYSTEM context ( and so the COM ) .

"I can add the manifest, but I also recently changed the way that the code underlying that factory works. I would be very surprised if it continues to work from SYSTEM."

That would be really downer :( adding the manifest would be cool or simply make the OS check "optional".
What changes have you done? Are they already in winget 1.9.preview included? I am using the latest "preview" build
Windows Package Manager (Preview) v1.9.1763-preview which still works and based on the commits the COM Security update was included in there. So it still works!

"The supported way to use winget COM from SYSTEM is with the in-process DLL. Using that, you would have control over the behavior of the version API via the manifest in your own executable."

I will look into the in-process DLL. is there any example / documentation of this? It was hard enough to handle the COM API like Marti did here. Without any idea / sample, we dont have a clue how to archive that. https://github.com/marticliment/WinGet-API-from-CSharp/tree/main/WindowsPackageManager%20Interop/WindowsPackageManager
And my thought was this approach already uses the In-process way. But he uses a different package.

Whats the difference here with this official package used in comparism to the in-proc version?https://www.nuget.org/packages/Microsoft.WindowsPackageManager.ComInterop#readme-body-tab

i tried a early approach via the https://github.com/JohnMcPMS/winget-cli/blob/master/src/Microsoft.Management.Deployment.Projection/Initializers/ActivationFactoryInstanceInitializer.cs stuff and the InProc specific class IDs, BUT i always get "REGDB_E_CLASSNOTREG" and the CLSID (for example 2DDE4456-64D9-4673-8F7E-A4F19A2E6CC3 ) is not found in my registry, where as the outproc CLSID are there (C53A4F16-787E-42A4-B304-29EFFB4BF597)

I am open for any approach, i simply just dont know how to implement!

@JohnMcPMS
Copy link
Member

That would be really downer :( adding the manifest would be cool or simply make the OS check "optional".
What changes have you done? Are they already in winget 1.9.preview included?

They would be coming to you via nuget (winrtact.dll), so technically you could avoid them by not updating (or not updating the interop you are using).

The change is to use the registered package location to find the executable, which will not work from SYSTEM since the package is not registered for that user.

in-process DLL. is there any example / documentation of this?

Unfortunately I don't have any shipped solution here. The closest thing would be to look at https://github.com/microsoft/winget-cli/tree/7ea9bca4deebf50f1b4d8c7092e61e1339d2830f/src/Microsoft.Management.Deployment.Projection and specifically https://github.com/microsoft/winget-cli/blob/7ea9bca4deebf50f1b4d8c7092e61e1339d2830f/src/Microsoft.Management.Deployment.Projection/Initializers/ActivationFactoryInstanceInitializer.cs

@JohnMcPMS
Copy link
Member

I've also created that PR for the manifest since it was very easy and shouldn't hurt anything. That is not an endorsement of support, just that it is the correct thing to do. You should still try to move to in-proc.

@PatrickSchmidtSE
Copy link
Author

PatrickSchmidtSE commented Jul 3, 2024

They would be coming to you via nuget (winrtact.dll), so technically you could avoid them by not updating (or not updating the interop you are using).

The change is to use the registered package location to find the executable, which will not work from SYSTEM since the package is not registered for that user.

I see, my current workaround already is based on the detection mechanic within the ManualServerActivation and "giving" the method what it wants. It would be interesting to see if there is some possibility to also make it seem "registered" for the USER and locate to the "correct" working executables.

"cli/tree/7ea9bca4deebf50f1b4d8c7092e61e1339d2830f/src/Microsoft.Management.Deployment.Projection and specifically https://github.com/microsoft/winget-cli/blob/7ea9bca4deebf50f1b4d8c7092e61e1339d2830f/src/Microsoft.Management.Deployment.Projection/Initializers/ActivationFactoryInstanceInitializer.cs"

I did look into the given stuff you posted yesterday already before, since like you said its the closest to anything. And i tried to make it work and i understand in-proc is the way.

I created the WIngetFactory exactly like that and filled it with the ActivationFactoryInstanceInstaller. I updated the ClassesDefinition with the "inproc" ones.

BUT i always get "REGDB_E_CLASSNOTREG" wenn calling "CreateInstance within ActivationFactoryInstanceInstaller" and the CLSID (for example 2DDE4456-64D9-4673-8F7E-A4F19A2E6CC3 ) is not found in my registry either so the error is correct, where as the outproc CLSID are there in my registry(C53A4F16-787E-42A4-B304-29EFFB4BF597) and are loaded correctly via the ManualActivation way

  1. Updates CLSIDs
    image

2)Implementation like found Microsoft.Management.Deployment.Projection/Initializers/ActivationFactoryInstanceInitializer.cs, error is happening then within CreatePackageManager
image
image

  1. The Exception error within WinRt.cs
    image

So the question is, if In-proc is the solution, why are the CLSIDs not registered on my system? How can i use CreateInstance with the ActivationFactoryInstanceInstaller.
Especially if the Tests found within GIT seem to simply use it that way...
https://github.com/microsoft/winget-cli/blob/7ea9bca4deebf50f1b4d8c7092e61e1339d2830f/src/AppInstallerCLIE2ETests/Interop/InstallInterop.cs

It would be really great if you by any chance could update a short sample ( or someone from your team), if the other COM way is unsupported . This would be a great benefit to the community using winget and promoting this great tool :)

Sidenote
Just as a sidenote. Is it planned that the "[Microsoft.DesktopAppInstaller_8wekyb3d8bbwe.msixbundle]" could be installed via Add-AppxPackage in SYSTEM context? I know the problem is the user profile in SYSTEM, but running "Get-AppxPackage" as Local System shows some packages that are there ( like itunes etc, so there must be a "solution").

@JohnMcPMS
Copy link
Member

For the in-proc case, did you have Microsoft.Management.Deployment.dll (which is the output of the Microsoft.Management.Deployment.InProc project, renamed) placed next to your .exe? I believe that it is also in the nuget I linked before.

Registering for SYSTEM is not supported as far as I know, other than a few select frameworks. I can ask about it again though in case things have changed.

JohnMcPMS added a commit that referenced this issue Jul 3, 2024
Related to #4589 

## Change
Applies the existing shared manifest to the COM server executable.
@PatrickSchmidtSE
Copy link
Author

PatrickSchmidtSE commented Jul 4, 2024

That would be great (regarding package registration) , like you said, for this check this would stop my workaround / approach. It would be therefore good to make this packagePath "skippable" ?. I know the solution is unsupported , but it works that way in SYSTEM context and would help in this case ( perhaps together with the manifest issue ).

Regarding the "Microsoft.Management.Deployment.dll" , No it seems only the .winmd file was deployed there. Also no such .dll is in the package, only the .winmdb file
image
image

Will make sure the all the .dll are also placed where the .exe path is and will report.

EDIT:
No, .dlls all there from the nuget package, still same "REGDB_E_CLASSNOTREG" , but thanks for suggesting this.
image

@JohnMcPMS
Copy link
Member

Renaming Microsoft.Management.Deployment.InProc.dll to Microsoft.Management.Deployment.dll is that I was suggesting as the option.

@PatrickSchmidtSE
Copy link
Author

PatrickSchmidtSE commented Jul 9, 2024

Wow, i never came to THAT idea.
It actually works!

I tested, scanning/finding packages, install, uninstall, UPGRADING the package, no "NoApplicableInstallers" :) NICE

Now i just need to make the Dll export/rename thing "releasable", but that should be managable.
Thank you @JohnMcPMS for your help on this one!!

@denelon denelon added this to the 1.9 Client milestone Jul 11, 2024
@denelon
Copy link
Contributor

denelon commented Jul 11, 2024

I'm going to go ahead and close this issue since it looks like there is a solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-COM-API Issue related to COM API Command-Upgrade Issue related to WinGet Upgrade Issue-Bug It either shouldn't be doing this or needs an investigation.
Projects
None yet
Development

No branches or pull requests

3 participants