Add upgrade functionality in Com api #1853
Conversation
| @@ -27,8 +27,7 @@ namespace AppInstaller::CLI | |||
| void COMInstallCommand::ExecuteInternal(Context& context) const | |||
| { | |||
| context << | |||
| Workflow::GetInstallerHash << | |||
| Workflow::VerifyInstallerHash << | |||
| Workflow::ReverifyInstallerHash << | |||
There was a problem hiding this comment.
nit: This seems overly aggressive on combining tasks. #Resolved
There was a problem hiding this comment.
We'll leave it as is for bug fix now
| @@ -341,6 +341,9 @@ namespace winrt::Microsoft::Management::Deployment::implementation | |||
| Microsoft::Management::Deployment::PackageVersionInfo packageVersionInfo = GetPackageVersionInfo(package, options); | |||
| AddPackageManifestToContext(packageVersionInfo, context.get()); | |||
|
|
|||
| // If the installer has dependencies, DependencySource needs to be set. | |||
There was a problem hiding this comment.
Is there not a way that we can set the actual Source object rather than adding an argument? #Resolved
There was a problem hiding this comment.
It's my test manifest is pointing to invalid dependencies. I'll remove this line as it's unnecessary.
| } | ||
| else | ||
| { | ||
| co_return GetInstallResult(executionStage, APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE, correlationData, false); |
There was a problem hiding this comment.
I think that we might need to break down the specific cases more for these errors. Specifically, I'm concerned about overloading this error (based on what I think is returning it, so I could be wrong). If the update does not have an applicable installer, do we get this error?
If the caller gives us a version that is less than or equal to the current version, should we have a different error? If the caller does not give us a version, but the latest version is already installed, what error will be produced, if any?
#Resolved
| winrt::Windows::Foundation::IAsyncOperationWithProgress<winrt::Microsoft::Management::Deployment::InstallResult, winrt::Microsoft::Management::Deployment::InstallProgress> | ||
| GetInstallProgress(winrt::Microsoft::Management::Deployment::CatalogPackage package, winrt::Microsoft::Management::Deployment::PackageCatalogInfo catalogInfo); | ||
| // Contract 4.0 |
There was a problem hiding this comment.
What happened to 3? Was that the enum value I added? #Resolved
ghost
added
Needs-Author-Feedback
| @@ -199,6 +199,8 @@ namespace winrt::Microsoft::Management::Deployment::implementation | |||
| resultStatus = winrt::Microsoft::Management::Deployment::InstallResultStatus::NoApplicableInstallers; | |||
| break; | |||
| case APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE: | |||
| case APPINSTALLER_CLI_ERROR_UPGRADE_VERSION_UNKNOWN: | |||
| case APPINSTALLER_CLI_ERROR_UPGRADE_VERSION_NOT_NEWER: | |||
There was a problem hiding this comment.
I'm not sure what the existing interactions look like, but we might need to separate these into new InstallResultStatus values as well.
It looks like we didn't split the "expected return codes" out in the InstallResultStatus, which itself seems like it might be an issue.
Change
Validation
Since the Com packaged tests are not ready yet, I manually verified the fixed Com install api and the newly added upgrade api works on wnget source and msstore rest source.
Next I'll work on enabling Com tests as part of the e2e tests.
Microsoft Reviewers: Open in CodeFlow