[1.7-stable] Fix: DeploymentManager failing with error package downgrade

[guimafelipe]

Replicating #6021

On current telemetry on experimental builds, the majority of the failures on the deployment initialize method is due to ERROR_INSTALL_PACKAGE_DOWNGRADE.

This PR tracks one of the possible ways of that happening in the current code.

When we call GetStatus() (either by itself or inside the Initialize() method), it tries to go over all the packages (Main and Singleton, in that order) and check if they are already installed or not (in the method VerifyPackage()).

The problem is that in the current way this is written, if the Main package is not installed in the machine, but the Singleton package is installed in a higher version, in the for loop, it will verify that Main is not installed and break from the loop. This break will make Singleton package not be verified, and it will not be added to the global map that tracks the versions of the currently installed packages.

Then, on the AddOrRegisterPackages() method in the Deploy stage, we will try to install both packages. But as Singleton was not added to the g_existingTargetPackagesIfHigherVersion map because of the early break, it will attempt to install it when a higher version is already installed.

A microsoft employee must use /azp run to validate using the pipelines below.

WARNING:
Comments made by azure-pipelines bot maybe inaccurate.
Please see pipeline link to verify that the build is being ran.

For status checks on the main branch, please use TransportPackage-Foundation-PR
(https://microsoft.visualstudio.com/ProjectReunion/_build?definitionId=81063&_a=summary)
and run the build against your PR branch with the default parameters.

[guimafelipe]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[DrusTheAxe]

Please update the description with info about the issue (not the autogenerated AI slop template)

[DrusTheAxe]

If Add|Stage|Register try to downgrade a package (e.g add v4 when v5 is present) they return HRESULT_FROM_WIN32(ERROR_INSTALL_PACKAGE_DOWNGRADE). Why not check for that and treat it as Success?

e.g. DeploymentManager.cpp line 393+

        if (useExistingPackageIfHigherVersion)
        {
            deploymentOperation = packageManager.RegisterPackageAsync(pathUri, nullptr, options);
        }
        else
        {
            deploymentOperation = packageManager.AddPackageAsync(pathUri, nullptr, options);
        }
...
    // Deployment reporting ERROR_INSTALL_PACKAGE_DOWNGRADE means a newer version of the package is already present
    // We should treat that as Success as deployment's fundamental model isn't Add(foo-v2.msix)=="install v2" but "install v2+"
    // Deployment doesn't guarantee an operating will install the exact version requested but rather the requested version
    // **or higher** (aka VersionSupercedence).
    if (deploymentOperationHResult == HRESULT_FROM_WIN32(ERROR_INSTALL_PACKAGE_DOWNGRADE))
    {
        deploymentOperationHResult  = S_OK;
    }
...
        // If deploymentOperationHResult indicates success, take that, ignore deploymentOperationExtendedHResult.
        // Otherwise, return deploymentOperationExtendedHResult if there is an error in it, if not, return deploymentOperationHResult.
        return SUCCEEDED(deploymentOperationHResult) ? deploymentOperationHResult :
            (FAILED(deploymentOperationExtendedHResult) ? deploymentOperationExtendedHResult : deploymentOperationHResult);

This is how PackageManager should have behaved but it was created in Win8 and VersionSupercedence behavior wasn't introduced until TH1, so for compat reasons PackageManager continues to return ERROR_INSTALL_PACKAGE_DOWNGRADE. Same reason that's additional information that should (by today's standards) be communicated via an [out] parameter but that would be a breaking change, so for compat reasons we continue returning the not-an-error HRESULT.

PackageDeploymentManager handles this the right way per modern standards. Look for ERROR_INSTALL_PACKAGE_DOWNGRADE in dev\PackageManager\API\M.W.M.D.PackageDeploymentManager.cpp for more details.

[DrusTheAxe]

See comment re ERROR_INSTALL_PACKAGE_DOWNGRADE

[guimafelipe]

If Add|Stage|Register try to downgrade a package (e.g add v4 when v5 is present) they return HRESULT_FROM_WIN32(ERROR_INSTALL_PACKAGE_DOWNGRADE). Why not check for that and treat it as Success?

e.g. DeploymentManager.cpp line 393+

        if (useExistingPackageIfHigherVersion)
        {
            deploymentOperation = packageManager.RegisterPackageAsync(pathUri, nullptr, options);
        }
        else
        {
            deploymentOperation = packageManager.AddPackageAsync(pathUri, nullptr, options);
        }
...
    // Deployment reporting ERROR_INSTALL_PACKAGE_DOWNGRADE means a newer version of the package is already present
    // We should treat that as Success as deployment's fundamental model isn't Add(foo-v2.msix)=="install v2" but "install v2+"
    // Deployment doesn't guarantee an operating will install the exact version requested but rather the requested version
    // **or higher** (aka VersionSupercedence).
    if (deploymentOperationHResult == HRESULT_FROM_WIN32(ERROR_INSTALL_PACKAGE_DOWNGRADE))
    {
        deploymentOperationHResult  = S_OK;
    }
...
        // If deploymentOperationHResult indicates success, take that, ignore deploymentOperationExtendedHResult.
        // Otherwise, return deploymentOperationExtendedHResult if there is an error in it, if not, return deploymentOperationHResult.
        return SUCCEEDED(deploymentOperationHResult) ? deploymentOperationHResult :
            (FAILED(deploymentOperationExtendedHResult) ? deploymentOperationExtendedHResult : deploymentOperationHResult);

This is how PackageManager should have behaved but it was created in Win8 and VersionSupercedence behavior wasn't introduced until TH1, so for compat reasons PackageManager continues to return ERROR_INSTALL_PACKAGE_DOWNGRADE. Same reason that's additional information that should (by today's standards) be communicated via an [out] parameter but that would be a breaking change, so for compat reasons we continue returning the not-an-error HRESULT.

PackageDeploymentManager handles this the right way per modern standards. Look for ERROR_INSTALL_PACKAGE_DOWNGRADE in dev\PackageManager\API\M.W.M.D.PackageDeploymentManager.cpp for more details.

This was my first thought, we could just make it accept ERROR_INSTALL_PACKAGE_DOWNGRADE as a success, but it would not fix the logic of the rest of the code that depends on the map g_existingTargetPackagesIfHigherVersion being correctly populated, used for example in DeployPackages to set it correctly to register to the present version of the package.

This fix makes the map to be correctly populated in every case.